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

Prerequisite is a set, ensure all dependencies are known/valid when reported #122

Merged
merged 9 commits into from
Oct 29, 2024

Conversation

gavofyork
Copy link
Owner

@gavofyork gavofyork commented Oct 26, 2024

Fixes #103

  • The WP/WR prerequisite is a set rather than an optional value.
  • The size of the set of a WR's prerequisites plus the size of said WR's SR lookup dictionary must total no more than 8.
  • All dependencies (defined as the set of prerequisites in union with the keys of the SR lookup dictionary) must have been recently reported at the time of reporting.
  • A mapping of WPH->SR is no longer retained in Accumulated.
  • A mapping of WPH->SR is maintained now in Recent Blocks ($\beta$).
  • SR lookups are validated immediately upon reporting. It is up to the block producer to ensure that they are legit since a block is invalid if the SR lookups cannot be validated.

@gavofyork gavofyork changed the title Prerequisite is a set, ensure all dependencies are known when reported Prerequisite is a set, ensure all dependencies are known/valid when reported Oct 27, 2024
Copy link
Contributor

@zdave-parity zdave-parity left a comment

Choose a reason for hiding this comment

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

Makes sense to me.

text/serialization.tex Outdated Show resolved Hide resolved
text/accumulation.tex Outdated Show resolved Hide resolved
text/accumulation.tex Outdated Show resolved Hide resolved
text/accumulation.tex Outdated Show resolved Hide resolved
text/accumulation.tex Outdated Show resolved Hide resolved
text/recent_history.tex Outdated Show resolved Hide resolved
text/reporting_assurance.tex Outdated Show resolved Hide resolved
text/reporting_assurance.tex Outdated Show resolved Hide resolved
text/reporting_assurance.tex Outdated Show resolved Hide resolved
text/reporting_assurance.tex Outdated Show resolved Hide resolved
@gavofyork gavofyork merged commit 1b0f010 into main Oct 29, 2024
@gavofyork gavofyork deleted the gav-deps-are-set branch October 29, 2024 20:50
@zdave-parity
Copy link
Contributor

Hmm one thing that has just occurred to me is that with this change, it is possible for bad WPH->SR mappings to be accepted, because we are simply trusting the mappings from guaranteed reports. If you collaborate with another guarantor you can get bogus mappings in the recent blocks state without punishment by sharing bogus guarantees but refusing to send any shards out.

@zdave-parity
Copy link
Contributor

To elaborate a bit, I'm thinking something along the following lines could happen:

  • There is a work-package A which exports some segments.
  • There is a work-package B which imports some segments from A by specifying A's hash.
  • There are two collaborating validators assigned to the same core who would like to tamper with the data imported by B.
  • A work-report X is produced for A, gets reported on chain, and accumulated.
  • Enough blocks pass such that A no longer appears in the recent blocks list.
  • The collaborating validators produce a bogus work-report X' claiming to be for work-package A but with a bogus segments-root and anchor (this bogus anchor just needs to be some recent block).
  • X' is shared and reported on chain. This is possible because the WP appears to be recent due to the bogus anchor, and not a duplicate as A has fallen out of the recent blocks list.
  • A work-report Y is produced for B which uses the bogus segments-root for A from work-report X'. The validators producing this work-report needn't necessarily be malicious; they could be honest validators that have seen work-report X' but not X.
  • Y is shared and reported on chain. Note that the bogus segments-root is accepted because it matches what was in work-report X', which is in the recent blocks list.
  • The collaborating validators do not share any shards for X' and so it eventually expires.
  • Y is made available, passes audit because it is valid, and gets accumulated because its dependencies are satisfied (A was previously accumulated).

@gavofyork
Copy link
Owner Author

Yup - so to fix this we need to ensure that the WP of incoming WRs not only don't appear in the Recent Blocks list but also not the Accumulated or Ready Queue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Guarantors should not get rewarded if their SR lookup is invalid
2 participants