fix: validate tx chain id #126
Conversation
WalkthroughThe changes enhance the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (4)
- bolt-sidecar/src/client/rpc.rs (1 hunks)
- bolt-sidecar/src/primitives/commitment.rs (1 hunks)
- bolt-sidecar/src/state/execution.rs (5 hunks)
- bolt-sidecar/src/state/fetcher.rs (2 hunks)
Additional comments not posted (8)
bolt-sidecar/src/primitives/commitment.rs (1)
46-53: LGTM! Add tests for the new method.The
validate_chain_idmethod is correctly implemented.However, ensure that unit tests are added to verify its functionality.
bolt-sidecar/src/state/fetcher.rs (2)
42-42: LGTM!The
get_chain_idmethod is correctly added to theStateFetchertrait.
183-185: LGTM! Add tests for the new method.The
get_chain_idmethod is correctly implemented in theStateClientstruct.However, ensure that unit tests are added to verify its functionality.
bolt-sidecar/src/client/rpc.rs (1)
36-39: LGTM! Add tests for the new method.The
get_chain_idmethod is correctly implemented in theRpcClientstruct.However, ensure that unit tests are added to verify its functionality.
bolt-sidecar/src/state/execution.rs (4)
40-42: LGTM!The
ChainIdMismatcherror variant is correctly added to theValidationErrorenum.
85-86: LGTM!The
chain_idfield is correctly added to theExecutionStatestruct.
117-120: LGTM!The
chain_idmethod is correctly implemented in theExecutionStatestruct.
151-154: LGTM! Add tests for the new validation logic.The chain ID validation logic is correctly implemented in the
try_commitmethod.However, ensure that unit tests are added to verify its functionality.
| slot: 0, | ||
| account_states: HashMap::new(), | ||
| block_templates: HashMap::new(), | ||
| chain_id: client.get_chain_id().await?, | ||
| client, | ||
| }) |
There was a problem hiding this comment.
Another way to do this can be to pass the chain ID value from config as a parameter while initializing the execution state, which felt to me as a dirty/ un-clean way.
There was a problem hiding this comment.
Yeah this is cleaner, great job
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- bolt-sidecar/src/state/execution.rs (5 hunks)
Additional comments not posted (5)
bolt-sidecar/src/state/execution.rs (5)
40-42: LGTM! ValidationError enum extended with ChainIdMismatchThe addition of
ChainIdMismatchtoValidationErroris appropriate for handling chain ID mismatches.
85-86: LGTM! ExecutionState struct extended with chain_idThe addition of the
chain_idfield toExecutionStateis appropriate for managing chain ID validation.
102-102: LGTM! ExecutionState::new method initializes chain_idThe initialization of the
chain_idfield in thenewmethod is appropriate for ensuring that thechain_idis set when theExecutionStateis created.
117-119: LGTM! ExecutionState::chain_id method addedThe addition of the
chain_idmethod is appropriate for retrieving thechain_idfield.
133-136: LGTM! ExecutionState::try_commit method validates chain_idThe addition of the chain ID validation in the
try_commitmethod is appropriate for ensuring that transactions are processed with the correct chain ID.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- bolt-sidecar/src/state/execution.rs (4 hunks)
Additional comments not posted (2)
bolt-sidecar/src/state/execution.rs (2)
40-42: Good addition ofChainIdMismatchvariant.The
ChainIdMismatchvariant is correctly added to theValidationErrorenum with an appropriate error message.
85-86: Good addition ofchain_idfield.The
chain_idfield is correctly added to theExecutionStatestruct and initialized in thenewmethod.Also applies to: 102-102
fixes #124
Summary by CodeRabbit
New Features
get_chain_id).InclusionRequest.ExecutionStatewith corresponding retrieval method.Improvements
ChainIdMismatchvariant inValidationError.Bug Fixes
ExecutionState.These updates enhance the reliability and functionality of the system by improving chain ID management and validation processes.