Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep multiple latest confirmed nonces at source in messages relay #719

Conversation

svyatonik
Copy link
Contributor

found this when working on #680

So there's lag before finalized source header H appear at target chain. Relay was reading latest_confirmed_nonce_at_source from best finalized source block. Then it has been using this value to compute number of headers that may fit into delivery transaction and was generating proof of this value at block H', which is best finalized source block, known to target node. If H != H', then the value may also be different => the target chain may have rejected some messages from delivery transaction. Relayer then had to wait for stall_timeout to pass and only then continue its work.

Another change is splitting single target_nonces_updated into best_target_nonces_updated and finalized_target_nonces_updated to maintain this new deque (latest_confirmed_nonces_at_source).

Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gave it a brief look, but it looks good to me.

Do I understand correctly, that this simply makes the relayer more resilient to cases where finality is taking a little bit longer to deliver (i.e. we can still send the part of the messages that we are supposed to send)?

@tomusdrw tomusdrw added the PR-audit-maybe Would be good to have the PR audited, but it doesn't look critical. label Feb 15, 2021
@svyatonik
Copy link
Contributor Author

Do I understand correctly, that this simply makes the relayer more resilient to cases where finality is taking a little bit longer to deliver (i.e. we can still send the part of the messages that we are supposed to send)?

It makes relayer aware that these lags exist - after this PR it'll always deliver maximal number of messages it can deliver. Before: it may have tried to deliver more messages than target chain may accept => some of messages were rejected. This difference was because of finality lagging (I'm talking here about our header chain finality, not the GRANDPA finality)

@svyatonik svyatonik merged commit e7a3128 into master Feb 15, 2021
@svyatonik svyatonik deleted the keep-multiple-latest-confirmed-nonces-at-source-in-messages-relay branch February 15, 2021 12:10
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
…ritytech#719)

* keep multiple latest confirmed nonces at source in messages relay

* post-merge fix
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
…ritytech#719)

* keep multiple latest confirmed nonces at source in messages relay

* post-merge fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-Message Delivery P-Relay PR-audit-maybe Would be good to have the PR audited, but it doesn't look critical.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants