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.

Sign up for GitHub

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

Jump to bottom

consensus roothash events specialization #588

Merged
merged 22 commits into from
Mar 1, 2024
Merged

consensus roothash events specialization #588

merged 22 commits into from
Mar 1, 2024

Conversation

pro-wh
Copy link
Collaborator

@pro-wh pro-wh commented Dec 16, 2023 •
edited
Loading

roothash events with related runtime and related runtime round

@pro-wh pro-wh force-pushed the pro-wh/feature/roothash branch 3 times, most recently from e0a9684 to 66c1cb2 Compare January 5, 2024 01:29
@pro-wh pro-wh changed the title (unfinished) consensus roothash events specialization consensus roothash events specialization Jan 5, 2024
@pro-wh
Copy link
Collaborator Author

pro-wh commented Jan 5, 2024

cutting this off here, moving messages to a new PR

@pro-wh pro-wh marked this pull request as ready for review January 5, 2024 01:29
@pro-wh
Copy link
Collaborator Author

pro-wh commented Jan 5, 2024

the fact that there are no regression test breakages 💀

Andrew7234
Andrew7234 approved these changes Jan 8, 2024
View reviewed changes
storage/oasis/nodeapi/file/consensus.go Outdated Show resolved Hide resolved
@mitjat
Copy link
Contributor

mitjat commented Jan 9, 2024

the fact that there are no regression test breakages 💀

That's unexpected IMO. The PR modifies the ConsensusEvent openapi type, which is the return type of /consensus/events, which is in run.sh. Worth looking into more.

mitjat
mitjat reviewed Jan 9, 2024
View reviewed changes
api/spec/v1.yaml Show resolved Hide resolved
api/spec/v1.yaml Outdated Show resolved Hide resolved
analyzer/consensus/consensus.go Outdated Show resolved Hide resolved
analyzer/consensus/runtimes.go Show resolved Hide resolved
api/spec/v1.yaml Show resolved Hide resolved
storage/oasis/nodeapi/api.go Outdated Show resolved Hide resolved
storage/oasis/nodeapi/api.go Outdated Show resolved Hide resolved
storage/migrations/07_roothash.up.sql Outdated Show resolved Hide resolved
api/spec/v1.yaml Show resolved Hide resolved
storage/oasis/nodeapi/cobalt/convert.go Show resolved Hide resolved
Base automatically changed from pro-wh/feature/holders20 to main January 13, 2024 00:27
@pro-wh pro-wh force-pushed the pro-wh/feature/roothash branch 2 times, most recently from 84ebeff to eeacb93 Compare January 26, 2024 01:23
@pro-wh pro-wh force-pushed the pro-wh/feature/roothash branch 2 times, most recently from 38f1457 to 2b0f121 Compare February 1, 2024 23:34
@pro-wh pro-wh requested a review from mitjat February 1, 2024 23:34
@pro-wh pro-wh force-pushed the pro-wh/feature/roothash branch 2 times, most recently from bb67732 to 20a246b Compare February 2, 2024 23:01
pro-wh and others added 12 commits February 27, 2024 14:50
@pro-wh
storage: specialize roothash message events
de71c92
@pro-wh
storage: handle roothash.in_msg_processed consensus events
2d09cea
@pro-wh @mitjat
storage: retain roothash event runtime and round
de6de13 
Co-authored-by: mitjat <[email protected]>
@pro-wh
storage: add roothash event columns and indexes
f832e88
@pro-wh
consensus: add RuntimeFromId
437b9b4
@pro-wh
consensus: index roothash related runtime and round
b44bc73
@pro-wh
consensus: add node id preimage todo
f99f123
@pro-wh
api: add related runtime and round fields to consensus events
3c418c1
@pro-wh
coreapi: add ExecutionDiscrepancyDetectedEvent.Round
d97f6dc 
cross-reference: oasisprotocol/oasis-core#5431

Adding this as nilable so we can detect versions below 23.0.3 leaving it out.
@pro-wh
storage: get new discrepancy round
52501fe
@pro-wh
storage: bump cache keys for roothash events
5ee212f
@pro-wh
storage: split convertEvent
0b4f5e9
@pro-wh pro-wh force-pushed the pro-wh/feature/roothash branch 2 times, most recently from 660762c to 8e79657 Compare February 28, 2024 00:12
@pro-wh pro-wh force-pushed the pro-wh/feature/roothash branch 2 times, most recently from 3be6019 to 6d1807d Compare February 28, 2024 00:32
@pro-wh
Copy link
Collaborator Author

pro-wh commented Feb 28, 2024

ok lots of unexplained cache changes, gotta figure this out

@pro-wh
Copy link
Collaborator Author

pro-wh commented Feb 28, 2024

more discussion on the new field names: change them all to roothash_, no more related_. because there's no desire to generalize this to other event types

@pro-wh
Copy link
Collaborator Author

pro-wh commented Feb 28, 2024

ok conclusions on other damask e2e cache changes:

  • emerald is currently disabled, and an empty cache has its hash seed reset each time. ignoring the diff
  • sourcify cache incorrectly ignored existing entries due to indexes being missing. I think I can resolve by regenerating the indexes
  • sourcify cache misc files missing is unintended. I'll regenerate and add
  • lock file added to sourcify cache is because we don't cleanly shut down the cache from the http proxy. ignoring the diff
  • event ordering is known not to have sufficient ORDER BY in api query. gonna push the diff and we can resolve separately

@pro-wh
Copy link
Collaborator Author

pro-wh commented Feb 29, 2024

ok test caches should be set. next is the renames

@pro-wh pro-wh merged commit bb32699 into main Mar 1, 2024
10 checks passed
@pro-wh pro-wh deleted the pro-wh/feature/roothash branch March 1, 2024 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mitjat mitjat mitjat approved these changes

@Andrew7234 Andrew7234 Andrew7234 approved these changes

@ptrus ptrus Awaiting requested review from ptrus ptrus is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pro-wh @mitjat @Andrew7234