feat!: deploy Blobstream at a custom nonce #255

rach-id · 2023-11-03T08:42:01Z

Overview

Closes #251

Checklist

New and updated code has appropriate documentation
New and updated code has new and/or updated testing
Required CI checks are passing
Visual proof for any user facing features like CLI or documentation updates
Linked issues closed with keywords

Summary by CodeRabbit

New Features
- Introduced a new function to compute a domain-separated hash using the validator set hash, nonce, and power threshold.
Bug Fixes
- Updated the initialize function in the Blobstream contract to use the computed checkpoint instead of the original hash value.
Tests
- Added a new function testDeployContractAtCustomNonce() and made adjustments to nonce values in various test functions.
Refactor
- Renamed _validatorSetHash to _validatorSetCheckpoint in the Blobstream contract and removed the ValidatorSetUpdatedEvent.

coderabbitai · 2023-11-03T08:42:09Z

Walkthrough

The changes primarily revolve around the introduction of a new concept, validatorSetCheckpoint, replacing the previous validatorSetHash. This is reflected in the initialize function of the Blobstream contract and in the test files. A new function domainSeparateValidatorSetHash is introduced in multiple files to compute this checkpoint. The nonce values in the test files have also been updated.

Changes

File Path	Change Summary
`src/Blobstream.sol`	Renamed `_validatorSetHash` to `_validatorSetCheckpoint` in `initialize` function. Removed `ValidatorSetUpdatedEvent`.
`src/lib/verifier/test/...`	Introduced `checkpoint` variable and `domainSeparateValidatorSetHash` function. Updated `initialize` function of `bridge` contract to use `checkpoint`.
`src/test/Blobstream.t.sol`	Updated nonce values in multiple functions. Added `testDeployContractAtCustomNonce` function.
`src/test/BlobstreamBenchmark.t.sol`	Added `domainSeparateValidatorSetHash` function. Updated `initialize` function of `Benchmark` contract to use `checkpoint`.

Poem

🐇 Hopping through the code, what do we see? 🍂

A change in the season, a change in the tree. 🌳

From validatorSetHash to checkpoint we leap, 🌌

While the moon above our codebase does peep. 🌙

Nonce values updated, one by one, 📈

Under the November's setting sun. 🌇

Celebrating this day in history, with joy and fun, 🎉

For on this day, our code's new journey has begun. 🚀

Tips

Chat with CodeRabbit Bot (`@coderabbitai`)

If you reply to a review comment from CodeRabbit, the bot will automatically respond.
To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
Note: Review comments are made on code diffs or files, not on the PR overview.
Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai help to get help.
@coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

src/Blobstream.sol

coderabbitai

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 856a9e5 and 2a01b7a.

Files selected for processing (5)

src/Blobstream.sol (1 hunks)
src/lib/verifier/test/DAVerifier.t.sol (2 hunks)
src/lib/verifier/test/RollupInclusionProofs.t.sol (2 hunks)
src/test/Blobstream.t.sol (4 hunks)
src/test/BlobstreamBenchmark.t.sol (2 hunks)

Files skipped from review due to trivial changes (2)

src/Blobstream.sol
src/test/BlobstreamBenchmark.t.sol

Additional comments: 9

src/lib/verifier/test/RollupInclusionProofs.t.sol (2)

214-216: The initialize function now uses a domain-separated hash checkpoint instead of the original validatorSetHash. Ensure that this change is reflected in all calls to initialize throughout the codebase.

370-379: The domainSeparateValidatorSetHash function is introduced to compute a domain-separated hash using the validator set hash, nonce, and power threshold. This function seems to be correctly implemented, but it would be good to add some unit tests to verify its behavior.

src/lib/verifier/test/DAVerifier.t.sol (2)

80-82: The initialize function of the bridge contract is now called with the checkpoint value instead of the hash value. Ensure that the initialize function in the bridge contract has been updated to accept this new argument and that it is being used correctly within the function.

202-211: The domainSeparateValidatorSetHash function is a new addition that computes a domain-separated hash using the validator set hash, nonce, and power threshold. This function seems to be implemented correctly, but it would be good to add some tests to ensure its correctness.

src/test/Blobstream.t.sol (5)

33-40: The setUp() function has been updated to initialize initialVelsetNonce to 1 instead of 0. Ensure that this change is reflected in all calls to setUp() throughout the codebase.

43-52: The testUpdateValidatorSet() function has been updated to initialize initialVelsetNonce and newNonce to 1 and 2 respectively, instead of 0 and 1. Ensure that this change is reflected in all calls to testUpdateValidatorSet() throughout the codebase.

70-74: The testSubmitDataRootTupleRoot() function has been updated to initialize initialVelsetNonce and nonce to 1 and 2 respectively, instead of 0 and 1. Ensure that this change is reflected in all calls to testSubmitDataRootTupleRoot() throughout the codebase.

92-119: A new function testDeployContractAtCustomNonce() has been added. This function tests the deployment of the contract at a custom nonce. Ensure that this function is called in the appropriate test cases.

131-134: The testVerifyAttestation() function has been updated to initialize initialVelsetNonce and nonce to 1 and 2 respectively, instead of 0 and 1. Ensure that this change is reflected in all calls to testVerifyAttestation() throughout the codebase.

coderabbitai

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 2a01b7a and 7213c87.

Files selected for processing (1)

wrappers/Blobstream.sol/wrapper.go (2 hunks)

Files not summarized due to errors (1)

wrappers/Blobstream.sol/wrapper.go: Error: Message exceeds token limit

Files not reviewed due to errors (1)

wrappers/Blobstream.sol/wrapper.go (Error: diff too large)

rach-id · 2023-11-03T08:49:20Z

src/Blobstream.sol

-
-        // LOGS
-
-        emit ValidatorSetUpdatedEvent(_nonce, _powerThreshold, _validatorSetHash);


[Note to reviewers]
removes emitting this event since we no longer have the validator set hash but the checkpoint. However, in other updates, we have the hash, so not sure how important this event is

rach-id · 2023-11-03T08:50:06Z

src/test/Blobstream.t.sol

@@ -89,6 +89,35 @@ contract RelayerTest is DSTest {
        assertEq(bridge.state_dataRootTupleRoots(nonce), newTupleRoot);
    }

+    function testDeployContractAtCustomNonce() public {
+        uint256 initialVelsetNonce = 1;
+        uint256 targetNonce = 200;


[Note to reviewers]
deploying starting nonce 200 while the valset is of nonce 1. No need to add a test also for submitting valset changes in this scenario since its the same as submitting data commitments

src/Blobstream.sol

Co-authored-by: John Adler <[email protected]>

coderabbitai

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 7213c87 and 942fc10.

Files selected for processing (1)

src/Blobstream.sol (1 hunks)

Files skipped from review due to trivial changes (1)

src/Blobstream.sol

coderabbitai

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 942fc10 and 98afb62.

Files selected for processing (1)

wrappers/Blobstream.sol/wrapper.go (2 hunks)

Files not summarized due to errors (1)

wrappers/Blobstream.sol/wrapper.go: Error: Message exceeds token limit

Files not reviewed due to errors (1)

wrappers/Blobstream.sol/wrapper.go (Error: diff too large)

feat!: deploy Blobstream at a custom nonce

2a01b7a

rach-id added contracts breaking labels Nov 3, 2023

rach-id requested a review from evan-forbes November 3, 2023 08:42

rach-id self-assigned this Nov 3, 2023

rach-id requested a review from adlerjohn as a code owner November 3, 2023 08:42

github-advanced-security bot found potential problems Nov 3, 2023

View reviewed changes

src/Blobstream.sol Dismissed Show resolved Hide resolved

src/Blobstream.sol Dismissed Show dismissed Hide dismissed

src/Blobstream.sol Dismissed Show dismissed Hide dismissed

chore: generate wrappers

7213c87

coderabbitai bot reviewed Nov 3, 2023

View reviewed changes

rach-id commented Nov 3, 2023

View reviewed changes

adlerjohn reviewed Nov 6, 2023

View reviewed changes

src/Blobstream.sol Outdated Show resolved Hide resolved

Update src/Blobstream.sol

942fc10

Co-authored-by: John Adler <[email protected]>

coderabbitai bot reviewed Nov 6, 2023

View reviewed changes

chore: regenerate wrappers

98afb62

rach-id requested a review from adlerjohn November 6, 2023 17:43

coderabbitai bot reviewed Nov 6, 2023

View reviewed changes

adlerjohn approved these changes Nov 6, 2023

View reviewed changes

rach-id merged commit 1fb7812 into celestiaorg:master Nov 6, 2023
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: deploy Blobstream at a custom nonce #255

feat!: deploy Blobstream at a custom nonce #255

rach-id commented Nov 3, 2023 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 3, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

coderabbitai bot left a comment

coderabbitai bot left a comment

rach-id Nov 3, 2023 •

edited

Loading

rach-id Nov 3, 2023 •

edited

Loading

coderabbitai bot left a comment

coderabbitai bot left a comment


		// LOGS

		emit ValidatorSetUpdatedEvent(_nonce, _powerThreshold, _validatorSetHash);

feat!: deploy Blobstream at a custom nonce #255

feat!: deploy Blobstream at a custom nonce #255

Conversation

rach-id commented Nov 3, 2023 • edited by coderabbitai bot Loading

Overview

Checklist

Summary by CodeRabbit

coderabbitai bot commented Nov 3, 2023 • edited Loading

Walkthrough

Changes

Poem

Chat with CodeRabbit Bot (@coderabbitai)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

rach-id Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

rach-id Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

rach-id commented Nov 3, 2023 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 3, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Configration File (`.coderabbit.yaml`)

rach-id Nov 3, 2023 •

edited

Loading

rach-id Nov 3, 2023 •

edited

Loading