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

Unordered Ancestry Proofs Cause Havok During Header Import #683

Closed
HCastano opened this issue Jan 27, 2021 · 2 comments
Closed

Unordered Ancestry Proofs Cause Havok During Header Import #683

HCastano opened this issue Jan 27, 2021 · 2 comments
Labels
A-feat New feature or request P-Runtime

Comments

@HCastano
Copy link
Contributor

While reviewing #672 @svyatonik brought up a good point about what would happen if we had
a situation in which an ancestry_proof consisted of unordered headers. To quote him:

So what I'm asking here is whether you have check if headers in ancestry_proof are
sorted (ASC)? Because after verifying the justification you're importing headers
assuming they come in-order. What I mean is that you're overwriting scheduled_change
in import_unchecked_header. So what disturbs me is that if AncestryChecker is able to
work with unordered ancestry proof + submitter provides unordered headers in
ancestry_proof, then we will be importing these headers out of order => there probably
will be situation when we will be able to overwrite scheduled_change. This seems
unexploitable though, in current conditions. But still - probably worth a check that
headers are ordered.

At the moment pallet-finality-verifier assumes that the headers in the ancestry proof
are in order without checking this fact itself. These headers are then imported without
verification into the base Substrate bridge pallet.

It is worth noting that this issue is not unique to ancestry proofs with unordered
headers, but also ones which consist of sparse headers.

We'll need a solution which can:

  • Provide flexibility in terms of ancestry proof verification (e.g ordered, unordered,
    sparse)
  • Allow for "full" header imports to the base bridge pallet (e.g import every single
    header, in order, eventually)
@tomusdrw
Copy link
Contributor

I'm slowly considering ditching the idea of the bridge pallet providing full header information for the entire chain.
This will not really be true with MMR (#263) nor if we switch to a more efficient ancestry checker using frame_system recent block hashes (#367), so I wonder if it's worth supporting at all.

Obviously it prevents some use cases (you need to rely on the latest on-chain state a bit more), but is much more efficient in terms of storage and computation.

While currently we're piggy-backing on the old pallet, during some refactoring we might be better designing around the more efficient option.

@tomusdrw
Copy link
Contributor

tomusdrw commented Mar 4, 2021

Closed via #755

@tomusdrw tomusdrw closed this as completed Mar 4, 2021
@tomusdrw tomusdrw added this to the Millau Testnet milestone Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-feat New feature or request P-Runtime
Projects
None yet
Development

No branches or pull requests

2 participants