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

feat(cosmic-swingset): add begin block check and transaction #8432

Merged
merged 3 commits into from
Oct 12, 2023

Conversation

mhofman
Copy link
Member

@mhofman mhofman commented Oct 4, 2023

closes: #8424

Description

This adds a consistency check for begin/end block, and does so through the host storage section of swing-store, which results in forcibly starting a block transaction at begin block to solve any future variations of #8423.

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

None

Testing Considerations

As usual, this part of cosmic-swingset relies on integration tests which should exercise the happy paths. The unhappy paths are wholly untested.

Upgrade Considerations

The enact upgrade may publish bundles, so it's also protected with a no-op state change to start a transaction.
The new saved state is fully backwards compatible, and will be created the first time needed (after an upgrade, or after a state-sync restore)

@mhofman mhofman changed the title feat(cosmic-swingset): add being block check and transaction feat(cosmic-swingset): add begin block check and transaction Oct 5, 2023
Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

Not sure if the comment is misleading (i.e. could use more clarification), or incorrect (i.e. should be rewritten). The code LGTM, though.

Please address the comment before merging.

Comment on lines 760 to 762
// Start a block transaction, but without changing state
// for the upcoming begin block check
saveBeginHeight(0);
Copy link
Member

Choose a reason for hiding this comment

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

Why "without changing state"? Saving a 0 is a state change, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

The default is 0 when missing, so writing 0 instead of defaulting does not effectively change the state. I'll calrify.

Copy link
Member Author

Choose a reason for hiding this comment

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

I could either re-write it as saveBeginHeight(savedBeginHeight); or update the comment, or both. Which one would be more clear?

@mhofman mhofman added the automerge:squash Automatically squash merge label Oct 12, 2023
@mergify mergify bot merged commit a9d113a into master Oct 12, 2023
71 checks passed
@mergify mergify bot deleted the mhofman/8424-tx-checks branch October 12, 2023 01:40
iomekam pushed a commit that referenced this pull request Oct 27, 2023
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mhofman added a commit that referenced this pull request Nov 8, 2023
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mhofman added a commit that referenced this pull request Nov 8, 2023
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mhofman added a commit that referenced this pull request Nov 10, 2023
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mhofman added a commit that referenced this pull request Jan 13, 2024
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mhofman added a commit that referenced this pull request Jan 15, 2024
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mhofman added a commit that referenced this pull request Jan 15, 2024
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mhofman added a commit that referenced this pull request Jan 15, 2024
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mhofman added a commit that referenced this pull request Jan 19, 2024
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mhofman added a commit that referenced this pull request Jan 19, 2024
* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
anilhelvaci pushed a commit to Jorge-Lopes/agoric-sdk that referenced this pull request Mar 4, 2024
…8432)

* feat(cosmic-swingset): add being block check and transaction

* fixup! feat(cosmic-swingset): add being block check and transaction

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Defensive check against unexpected commit points
2 participants