chore(ssz): Use the correct transactions root merkleization based on version #1791
Conversation
WalkthroughRecent changes enhance the clarity and maintainability of the codebase by introducing constants for blockchain chain IDs and refactoring functions to utilize these constants instead of hardcoded values. Additional modifications improve transaction handling by updating method signatures and parameters across various components, ensuring better flexibility and configurability in transaction processing. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Codebase
participant Constants
participant TransactionHandler
Developer->>Codebase: Implement constants for chain IDs
Codebase->>Constants: Define Devnet and Testnet IDs
Codebase->>TransactionHandler: Update methods to use constants
TransactionHandler->>Constants: Retrieve chain ID for processing
TransactionHandler->>TransactionHandler: Handle transactions flexibly
Poem
|
|
hood af but works, can you put some comments saying that this is a temporary hack |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1791 +/- ##
==========================================
- Coverage 26.35% 26.33% -0.03%
==========================================
Files 316 316
Lines 14213 14224 +11
Branches 27 27
==========================================
Hits 3746 3746
- Misses 10357 10366 +9
- Partials 110 112 +2
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (11)
- mod/config/pkg/spec/constants.go (1 hunks)
- mod/config/pkg/spec/devnet.go (1 hunks)
- mod/config/pkg/spec/testnet.go (1 hunks)
- mod/consensus-types/pkg/types/payload.go (3 hunks)
- mod/consensus-types/pkg/types/payload_test.go (1 hunks)
- mod/node-api/backend/mocks/beacon_block.mock.go (1 hunks)
- mod/node-api/backend/mocks/beacon_block_header.mock.go (1 hunks)
- mod/node-api/backend/mocks/deposit.mock.go (4 hunks)
- mod/state-transition/pkg/core/state_processor.go (3 hunks)
- mod/state-transition/pkg/core/state_processor_payload.go (1 hunks)
- mod/state-transition/pkg/core/types.go (2 hunks)
Additional comments not posted (70)
mod/config/pkg/spec/constants.go (2)
1-20: License Header Looks GoodThe license header is correctly included and follows the standard format.
21-29: Constants Declaration Looks GoodThe constants for
DevnetEth1ChainIDandTestnetEth1ChainIDare correctly declared and appropriately named.mod/config/pkg/spec/devnet.go (2)
Line range hint
1-1:
License Header Looks GoodThe license header is correctly included and follows the standard format.
39-39: Improved MaintainabilityReplacing the hardcoded value with
DevnetEth1ChainIDenhances maintainability and readability.mod/config/pkg/spec/testnet.go (2)
Line range hint
1-1:
License Header Looks GoodThe license header is correctly included and follows the standard format.
41-41: Improved MaintainabilityReplacing the hardcoded value with
TestnetEth1ChainIDenhances maintainability and readability.mod/node-api/backend/mocks/deposit.mock.go (22)
11-13: LGTM! The introduction of generics enhances flexibility and type safety.The
Depositstruct now accepts a type parameterDepositT, allowing it to be more versatile.
15-17: LGTM! The introduction of generics enhances flexibility and type safety.The
Deposit_Expecterstruct now accepts a type parameterDepositT, allowing it to be more versatile.
19-21: LGTM! The method update ensures type safety.The
EXPECTmethod now returns a pointer toDeposit_Expecter[DepositT].
Line range hint
24-38:
LGTM! The method update ensures type safety.The
GetIndexmethod now uses the new generic type parameter.
42-44: LGTM! The introduction of generics enhances flexibility and type safety.The
Deposit_GetIndex_Callstruct now accepts a type parameterDepositT, allowing it to be more versatile.
47-49: LGTM! The method update ensures type safety.The
GetIndexmethod inDeposit_Expecternow returns a pointer toDeposit_GetIndex_Call[DepositT].
51-56: LGTM! The method update ensures type safety.The
Runmethod inDeposit_GetIndex_Callnow uses the new generic type parameter.
58-60: LGTM! The method update ensures type safety.The
Returnmethod inDeposit_GetIndex_Callnow uses the new generic type parameter.
63-65: LGTM! The method update ensures type safety.The
RunAndReturnmethod inDeposit_GetIndex_Callnow uses the new generic type parameter.
Line range hint
69-95:
LGTM! The method update ensures type safety.The
MarshalSSZmethod now uses the new generic type parameter.
99-101: LGTM! The introduction of generics enhances flexibility and type safety.The
Deposit_MarshalSSZ_Callstruct now accepts a type parameterDepositT, allowing it to be more versatile.
104-106: LGTM! The method update ensures type safety.The
MarshalSSZmethod inDeposit_Expecternow returns a pointer toDeposit_MarshalSSZ_Call[DepositT].
108-113: LGTM! The method update ensures type safety.The
Runmethod inDeposit_MarshalSSZ_Callnow uses the new generic type parameter.
115-117: LGTM! The method update ensures type safety.The
Returnmethod inDeposit_MarshalSSZ_Callnow uses the new generic type parameter.
120-122: LGTM! The method update ensures type safety.The
RunAndReturnmethod inDeposit_MarshalSSZ_Callnow uses the new generic type parameter.
Line range hint
126-140:
LGTM! The method update ensures type safety.The
UnmarshalSSZmethod now uses the new generic type parameter.
144-146: LGTM! The introduction of generics enhances flexibility and type safety.The
Deposit_UnmarshalSSZ_Callstruct now accepts a type parameterDepositT, allowing it to be more versatile.
150-152: LGTM! The method update ensures type safety.The
UnmarshalSSZmethod inDeposit_Expecternow returns a pointer toDeposit_UnmarshalSSZ_Call[DepositT].
154-159: LGTM! The method update ensures type safety.The
Runmethod inDeposit_UnmarshalSSZ_Callnow uses the new generic type parameter.
161-163: LGTM! The method update ensures type safety.The
Returnmethod inDeposit_UnmarshalSSZ_Callnow uses the new generic type parameter.
166-168: LGTM! The method update ensures type safety.The
RunAndReturnmethod inDeposit_UnmarshalSSZ_Callnow uses the new generic type parameter.
173-180: LGTM! The method update enhances usability in tests.The
NewDepositfunction now accepts a type parameter for theDepositinstance it creates.mod/state-transition/pkg/core/state_processor_payload.go (1)
61-64: LGTM! The method update enhances the handling of transaction types and improves contextual integrity.The
ToHeadermethod call now includes additional parameters:sp.bartioTxsMerkleizer,sp.properTxsMerkleizer, andsp.cs.DepositEth1ChainID().mod/state-transition/pkg/core/types.go (1)
168-172: LGTM! The method update enhances flexibility and functionality.The
mod/consensus-types/pkg/types/payload_test.go (1)
206-208: LGTM! Ensure the new parameters are correctly handled in theToHeadermethod.The additional parameters in the
ToHeadermethod call enhance the flexibility of the payload processing. Verify that these parameters are correctly handled in theToHeadermethod.mod/state-transition/pkg/core/state_processor.go (2)
81-82: LGTM! Ensure the new fields are correctly used in the relevant methods.The new fields
bartioTxsMerkleizerandproperTxsMerkleizerenhance the transaction processing capabilities of theStateProcessor. Verify that these fields are correctly used in the relevant methods.
138-144: LGTM! Ensure the new fields are correctly initialized.The new fields
bartioTxsMerkleizerandproperTxsMerkleizerare correctly initialized in theNewStateProcessorfunction. Verify that the initialization logic is correct and complete.mod/node-api/backend/mocks/beacon_block_header.mock.go (11)
25-43: LGTM! Ensure the return value is correctly handled.The
GetBodyRootmethod provides a mock implementation for theGetBodyRootmethod of theBeaconBlockHeadertype. Verify that the return value is correctly handled.
72-90: LGTM! Ensure the return value is correctly handled.The
GetParentBlockRootmethod provides a mock implementation for theGetParentBlockRootmethod of theBeaconBlockHeadertype. Verify that the return value is correctly handled.
119-135: LGTM! Ensure the return value is correctly handled.The
GetProposerIndexmethod provides a mock implementation for theGetProposerIndexmethod of theBeaconBlockHeadertype. Verify that the return value is correctly handled.
164-180: LGTM! Ensure the return value is correctly handled.The
GetSlotmethod provides a mock implementation for theGetSlotmethod of theBeaconBlockHeadertype. Verify that the return value is correctly handled.
209-227: LGTM! Ensure the return value is correctly handled.The
GetStateRootmethod provides a mock implementation for theGetStateRootmethod of theBeaconBlockHeadertype. Verify that the return value is correctly handled.
256-284: LGTM! Ensure the return values are correctly handled.The
HashTreeRootmethod provides a mock implementation for theHashTreeRootmethod of theBeaconBlockHeadertype. Verify that the return values are correctly handled.
313-341: LGTM! Ensure the return values are correctly handled.The
MarshalSSZmethod provides a mock implementation for theMarshalSSZmethod of theBeaconBlockHeadertype. Verify that the return values are correctly handled.
370-385: LGTM! Ensure the parameters and return value are correctly handled.The
Newmethod provides a mock implementation for theNewmethod of theBeaconBlockHeadertype. Verify that the parameters and return value are correctly handled.
420-423: LGTM! Ensure the parameter is correctly handled.The
SetStateRootmethod provides a mock implementation for theSetStateRootmethod of theBeaconBlockHeadertype. Verify that the parameter is correctly handled.
453-469: LGTM! Ensure the parameter and return value are correctly handled.The
UnmarshalSSZmethod provides a mock implementation for theUnmarshalSSZmethod of theBeaconBlockHeadertype. Verify that the parameter and return value are correctly handled.
499-511: LGTM! Ensure the parameters and return value are correctly handled.The
NewBeaconBlockHeadermethod provides a mock implementation for creating a new instance ofBeaconBlockHeader. Verify that the parameters and return value are correctly handled.mod/consensus-types/pkg/types/payload.go (2)
Line range hint
557-587:
LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
ToHeadermatch the new signature.Verification successful
Verification successful: All function calls to
ToHeadermatch the new signature.
mod/state-transition/pkg/core/types.gomod/state-transition/pkg/core/state_processor_payload.gomod/consensus-types/pkg/types/payload.gomod/consensus-types/pkg/types/payload_test.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `ToHeader` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 'ToHeader'Length of output: 2642
557-560: Verify correctness of the conditional logic.The conditional logic based on
eth1ChainIDensures the correct merkleizer is used. However, verify thatspec.TestnetEth1ChainIDis correctly defined and used.Verification successful
Verify correctness of the conditional logic.
The conditional logic based on
eth1ChainIDensures the correct merkleizer is used. The definition and usage ofspec.TestnetEth1ChainIDare correct and consistent within the codebase.
spec.TestnetEth1ChainIDis defined inmod/config/pkg/spec/constants.gowith a value of80084.- It is used in the conditional logic in
mod/consensus-types/pkg/types/payload.go.- It is assigned in
mod/config/pkg/spec/testnet.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and usage of spec.TestnetEth1ChainID. # Test: Search for the definition and usage of spec.TestnetEth1ChainID. Expect: Correct definition and usage. rg --type go 'TestnetEth1ChainID'Length of output: 368
mod/node-api/backend/mocks/beacon_block.mock.go (24)
25-41: LGTM!The code changes are approved.
70-86: LGTM!The code changes are approved.
115-133: LGTM!The code changes are approved.
162-178: LGTM!The code changes are approved.
207-223: LGTM!The code changes are approved.
252-270: LGTM!The code changes are approved.
299-315: LGTM!The code changes are approved.
344-372: LGTM!The code changes are approved.
401-404: LGTM!The code changes are approved.
434-450: LGTM!The code changes are approved.
480-496: LGTM!The code changes are approved.
525-537: LGTM!The code changes are approved.
70-86: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Verify correctness of type assertions.
Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
- The type assertions in the provided code snippet are correct and follow the standard pattern for mock functions in Go. The code checks if the return value is a function and calls it if so; otherwise, it directly assigns the return value.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
25-41: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Verify correctness of type assertions.
The type assertions for the return value in the
GetBodymethod are consistent with other similar implementations in the codebase and handle potential panics appropriately.
mod/node-api/backend/mocks/beacon_block.mock.go: The type assertion forBeaconBlockBodyTis correctly implemented.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
299-315: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Type assertions in
IsNilmethod are correct.The type assertions for the return value in the
IsNilmethod are consistent with the patterns used in other mock methods and handle potential panics gracefully.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
115-133: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Verify correctness of type assertions.
The type assertions for the return value in the
GetParentBlockRootmethod are correct and consistent with the patterns used in other mock methods in the codebase.
mod/node-api/backend/mocks/beacon_block.mock.go: ret.Get(0).(bytes.B32)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
162-178: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Verify correctness of type assertions.
The type assertions in the
GetProposerIndexmethod are correct and consistent with other mock methods in the codebase. The implementation handles potential panics gracefully.
mod/node-api/backend/mocks/beacon_block.mock.go: Lines 162-178Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
480-496: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Verified correctness of type assertions.
The type assertions for the return value in the
Versionmethod are correct and handle potential panics gracefully.
mod/node-api/backend/mocks/beacon_block.mock.go:
- Line 490:
r0 = ret.Get(0).(uint32)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
525-537: Verify correctness of the initialization.Ensure that the initialization of the mock and the registration of the testing interface are correct.
344-372: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Verify correctness of type assertions.
The type assertions for the return values in the
MarshalSSZmethod appear to be correct and handle potential errors gracefully.
mod/node-api/backend/mocks/beacon_block.mock.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
434-450: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Type assertions in mock methods are correctly implemented.
The type assertions for the return value in the
UnmarshalSSZmethod are consistent with the patterns used in other mock methods and handle potential panics gracefully.
mod/node-api/backend/mocks/beacon_block.mock.golines 434-450Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
207-223: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Type assertions in the
GetSlotmethod are correct.The type assertions for the return value in the
GetSlotmethod are implemented correctly and follow the same pattern as other mock methods in the codebase.
- The type assertion checks if the returned value is a function, and if not, it directly assigns the value.
- This pattern is consistent and correctly implemented throughout the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
252-270: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
Verification successful
Type assertions in
GetStateRootmethod are correct.The type assertions for the return value in the
GetStateRootmethod are correctly implemented and handle potential panics gracefully.
mod/node-api/backend/mocks/beacon_block.mock.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of type assertions in mock methods. # Test: Search for type assertions in mock methods. Expect: Correct type assertions. rg --type go 'ret.Get\(0\)\.\(.*\)'Length of output: 26130
401-404: Verify correctness of type assertions.Ensure that the type assertions for the return value are correct and handle potential panics gracefully.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (3)
- mod/consensus-types/pkg/types/payload.go (3 hunks)
- mod/state-transition/pkg/core/state_processor.go (3 hunks)
- mod/state-transition/pkg/core/state_processor_payload.go (1 hunks)
Additional comments not posted (3)
mod/state-transition/pkg/core/state_processor.go (2)
144-150: LGTM!The constructor changes are straightforward and align with the new fields added to the struct.
80-88: Ensure proper documentation for temporary solution.The TODO comment indicates that
bartioTxsMerkleizeris a temporary solution. Add comments to clarify this in the code for future maintainability.+ // TODO: This is a temporary solution to avoid breaking changes. Needs to be hardforked off of. bartioTxsMerkleizer *merkle.Merkleizer[[32]byte, common.Root]Likely invalid or redundant comment.
mod/consensus-types/pkg/types/payload.go (1)
Line range hint
557-581:
Ensure proper error handling for transaction root calculation.The error handling for
txsRootErrshould be more explicit to provide better debugging information.return txsRootErr + if txsRootErr != nil { + return fmt.Errorf("failed to calculate transactions root: %w", txsRootErr) + }
Summary by CodeRabbit
New Features
DevnetEth1ChainIDandTestnetEth1ChainIDto centralize blockchain environment identifiers.BeaconBlock,BeaconBlockHeader, andDeposittypes to enhance unit testing capabilities.Enhancements
StateProcessortype for more sophisticated transaction data processing.Bug Fixes
Tests