refactor(storage): common inbound nonce #40
Conversation
WalkthroughThe changes involve updating nonce handling mechanisms across various contracts, particularly by replacing the Changes
Sequence Diagram(s)sequenceDiagram
participant Sender
participant GatewayStorage
participant BootstrapLzReceiver
participant ClientGatewayLzReceiver
Sender ->> BootstrapLzReceiver: Send message with nonce
BootstrapLzReceiver ->> GatewayStorage: Verify and update nonce
GatewayStorage ->> GatewayStorage: Validate nonce
GatewayStorage ->> BootstrapLzReceiver: Acknowledge update
BootstrapLzReceiver ->> Sender: Confirmation
Sender ->> ClientGatewayLzReceiver: Send message with nonce
ClientGatewayLzReceiver ->> GatewayStorage: Verify and update nonce
GatewayStorage ->> GatewayStorage: Validate nonce
GatewayStorage ->> ClientGatewayLzReceiver: Acknowledge update
ClientGatewayLzReceiver ->> Sender: Confirmation
Poem
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: CHILL
Files ignored due to path filters (2)
docs/architecture.svgis excluded by!**/*.svgdocs/lst-flow.svgis excluded by!**/*.svg
Files selected for processing (8)
- src/core/BootstrapLzReceiver.sol (2 hunks)
- src/core/ClientGatewayLzReceiver.sol (2 hunks)
- src/core/ExocoreGateway.sol (2 hunks)
- src/storage/BootstrapStorage.sol (1 hunks)
- src/storage/ClientChainGatewayStorage.sol (1 hunks)
- src/storage/ExocoreGatewayStorage.sol (1 hunks)
- src/storage/GatewayStorage.sol (2 hunks)
- test/mocks/ExocoreGatewayMock.sol (2 hunks)
Files skipped from review due to trivial changes (2)
- src/storage/BootstrapStorage.sol
- src/storage/ClientChainGatewayStorage.sol
Additional comments not posted (6)
src/storage/GatewayStorage.sol (2)
20-21: Introduction of public mappinginboundNonce.This mapping tracks nonces for specific sources, which is crucial for ensuring the integrity and non-repudiation of messages across different chains. This change aligns with the PR's goal to centralize nonce management.
30-36: Implementation of_verifyAndUpdateNoncefunction.This function correctly implements nonce verification and updates the nonce if it matches the expected value. The use of
revertwith a custom error message is a best practice for handling unexpected nonces, providing clear feedback on the error.src/core/BootstrapLzReceiver.sol (1)
21-21: Integration of_verifyAndUpdateNoncein_lzReceive.The integration of
_verifyAndUpdateNoncewithin_lzReceiveensures that every incoming message is verified for nonce correctness before processing. This is a critical security feature that prevents replay attacks.src/core/ClientGatewayLzReceiver.sol (1)
29-29: Integration of_verifyAndUpdateNoncein_lzReceive.The use of
_verifyAndUpdateNoncehere ensures that each message processed by the client gateway has its nonce verified, which is essential for maintaining the integrity and sequence of cross-chain messages.
[APROVED]test/mocks/ExocoreGatewayMock.sol (1)
170-170: Integration of_verifyAndUpdateNoncein_lzReceivefor mock testing.Correctly integrating
_verifyAndUpdateNoncein the mock contract's_lzReceivefunction ensures that the nonce verification logic is consistently tested across different implementations, maintaining the integrity of the testing process.src/core/ExocoreGateway.sol (1)
149-149: Refactored Nonce Verification and Update ApprovedThe replacement of
_consumeInboundNoncewith_verifyAndUpdateNoncealigns with the PR's objectives to centralize and improve nonce management. Ensure that the new function_verifyAndUpdateNonceis thoroughly tested and audited to prevent security vulnerabilities such as replay attacks.Verification successful
Refactored Nonce Verification and Update Approved
The replacement of
_consumeInboundNoncewith_verifyAndUpdateNoncealigns with the PR's objectives to centralize and improve nonce management. The implementation of_verifyAndUpdateNonceinsrc/storage/GatewayStorage.solcorrectly verifies and updates the nonce, ensuring it prevents replay attacks. Its usage in other files (src/core/BootstrapLzReceiver.sol,src/core/ClientGatewayLzReceiver.sol,src/core/ExocoreGateway.sol, andtest/mocks/ExocoreGatewayMock.sol) is consistent and appropriate.
- Implementation Location:
src/storage/GatewayStorage.sol- Usage Locations:
src/core/BootstrapLzReceiver.solsrc/core/ClientGatewayLzReceiver.solsrc/core/ExocoreGateway.soltest/mocks/ExocoreGatewayMock.solScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of `_verifyAndUpdateNonce`. # Test: Search for the function implementation and usage. Expect: Secure and correct implementation. rg --type solidity $'_verifyAndUpdateNonce'Length of output: 75
Script:
#!/bin/bash # Description: Verify the implementation and usage of `_verifyAndUpdateNonce`. # Step 1: Find all Solidity files in the repository. SOL_FILES=$(fd --extension sol) # Step 2: Search for the `_verifyAndUpdateNonce` function definition and usage within these files. for file in $SOL_FILES; do echo "Searching in $file" rg '_verifyAndUpdateNonce' "$file" doneLength of output: 18923
Script:
#!/bin/bash # Description: Extract the implementation of `_verifyAndUpdateNonce` and review its usage. # Step 1: Extract the implementation of `_verifyAndUpdateNonce` from `src/storage/GatewayStorage.sol`. echo "Implementation of _verifyAndUpdateNonce in src/storage/GatewayStorage.sol:" rg --context 10 'function _verifyAndUpdateNonce' src/storage/GatewayStorage.sol # Step 2: Review the usage of `_verifyAndUpdateNonce` in other files. echo "Usage of _verifyAndUpdateNonce in other files:" rg '_verifyAndUpdateNonce' src/core/BootstrapLzReceiver.sol src/core/ClientGatewayLzReceiver.sol src/core/ExocoreGateway.sol test/mocks/ExocoreGatewayMock.solLength of output: 1694
while the overrides still need to be present to inform the compiler about the base class whose function should be called, the implementation can still be made common in the base class.
|
The last commit a40ff77 moves the implementation of |
This reverts commit a40ff77.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/storage/ClientChainGatewayStorage.sol (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/storage/ClientChainGatewayStorage.sol
The mapping of
eid->sender->noncewas stored previously in allBootstrapStorageandExocoreGatewayStorage. This change moves that mapping into the parentGatewayStoragecontract, as well as its associated validation functionality.The PR removes the TSS-related nonce that is no longer used.
Summary by CodeRabbit
Bug Fixes
Documentation