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

fix(block_production): dynamic block closing #456

Merged
merged 4 commits into from
Jan 8, 2025

Conversation

Mohiiit
Copy link
Contributor

@Mohiiit Mohiiit commented Jan 8, 2025

Summary

This PR ensures that madara correctly adds a special address to the state diff when dynamically closing a block with a number greater than 10. The special address uses the current block number minus 10 as the key and the hash of (current block - 10) as the value. This is required for the SNOS.

Pull Request Type

  • Bugfix

Current Behavior

When the bouncer configuration reaches its limit, Madara dynamically closes the block. However, during this process, the required special address is not added to the state diff for blocks numbered greater than 10.

New Behavior

When closing a block dynamically, if the block number is greater than 10, a special address is now correctly added to the state diff with:

  • Key: Current block number - 10
  • Value: Hash of (current block - 10)

Breaking Changes

No breaking changes introduced.

Additional Notes

This fix ensures consistency in state diffs when dynamic block closing occurs, maintaining expected behavior for blocks greater than 10.

@Mohiiit Mohiiit self-assigned this Jan 8, 2025
@Mohiiit Mohiiit added bug Report an issue or unexpected behavior sequencer Related to the sequencing logic and implementation labels Jan 8, 2025
@@ -358,20 +358,45 @@ impl<Mempool: MempoolProvider> BlockProductionTask<Mempool> {
Ok(())
}

fn maybe_add_prev_block_hash(&self, state_diff: &mut StateDiff, block_n: u64) -> Result<(), Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a doc comment to this function explaining why this is needed and what it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

crates/madara/client/block_production/src/lib.rs Outdated Show resolved Hide resolved
@@ -358,20 +358,45 @@ impl<Mempool: MempoolProvider> BlockProductionTask<Mempool> {
Ok(())
}

fn maybe_add_prev_block_hash(&self, state_diff: &mut StateDiff, block_n: u64) -> Result<(), Error> {
Copy link
Member

Choose a reason for hiding this comment

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

The current name maybe_add_prev_block_hash is somewhat ambiguous and doesn't fully capture the function's role in the state management process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does it make more sense now? or any suggestion what can be improved?

Copy link
Member

Choose a reason for hiding this comment

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

perfect with documentation 👍

@Mohiiit Mohiiit requested review from Trantorian1 and jbcaron January 8, 2025 08:29
Copy link
Collaborator

@Trantorian1 Trantorian1 left a comment

Choose a reason for hiding this comment

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

Seems good! 👍

@jbcaron jbcaron changed the title Fix/dynamic block closing fix(block_production): dynamic block closing Jan 8, 2025
@antiyro antiyro merged commit 448bef5 into main Jan 8, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report an issue or unexpected behavior sequencer Related to the sequencing logic and implementation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants