test(consensus-types): add more test for block_denebplus #1742
Conversation
WalkthroughThe new tests in Changes
Poem
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 Configuration 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 (1)
- mod/consensus-types/pkg/types/block_denebplus_test.go (1 hunks)
Additional comments not posted (8)
mod/consensus-types/pkg/types/block_denebplus_test.go (8)
36-91: LGTM!The function
generateBeaconBlockDenebPluscorrectly generates aBeaconBlockDenebPlusinstance with hardcoded values for testing purposes.
93-96: LGTM!The function
TestBeaconBlockDenebPlus_Versioncorrectly tests the version retrieval method ofBeaconBlockDenebPlus.
98-104: LGTM!The function
TestBeaconBlockDenebPlus_IsNilcorrectly tests theIsNilmethod for both nil and non-nil instances ofBeaconBlockDenebPlus.
106-113: LGTM!The function
TestBeaconBlockDenebPlus_SetStateRootcorrectly tests theSetStateRootmethod by setting a new state root and verifying the equality.
115-120: LGTM!The function
TestBeaconBlockDenebPlus_GetBodycorrectly tests theGetBodymethod by verifying the body is not nil and comparing it with the raw body.
122-130: LGTM!The function
TestBeaconBlockDenebPlus_GetHeadercorrectly tests theGetHeadermethod by verifying the equality of various header fields.
132-226: LGTM!The function
TestBeaconBlockDenebPlus_MarshalSSZ_UnmarshalSSZcorrectly tests the SSZ marshaling and unmarshaling methods for various cases, including valid, empty, and invalid buffer size cases.
229-241: LGTM!The function
TestBeaconBlockDenebPlus_GetTreecorrectly tests theGetTreemethod by generating the tree, comparing the tree root with the expected root, and ensuring no errors occur.
…oa/add-test-block-denebplus
yeah, I just updated the test, thank you |
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)
- mod/consensus-types/pkg/types/block_denebplus_test.go (1 hunks)
Additional comments not posted (7)
mod/consensus-types/pkg/types/block_denebplus_test.go (7)
102-105: Test for version retrievalThe test
TestBeaconBlockDenebPlus_Versioncorrectly asserts that the version method ofBeaconBlockDenebPlusreturns the expected version. This is a straightforward test that effectively checks the functionality of the version method.
107-113: Test for nil checkThe test
TestBeaconBlockDenebPlus_IsNileffectively checks both theniland non-nilscenarios. It uses therequirepackage to assert the conditions, which is appropriate for unit tests where failing any assertion should stop the test immediately.
115-122: Test for setting the state rootThe test
TestBeaconBlockDenebPlus_SetStateRootcorrectly assigns a new state root and asserts the change. This test ensures that theSetStateRootmethod functions as expected. The use of a fixed byte array for the new root is appropriate for this test's scope.
124-129: Test for body retrievalThe test
TestBeaconBlockDenebPlus_GetBodyverifies that the body of the block is retrieved correctly. It also checks the equality of the retrieved body with the expected body, ensuring the getter's functionality is intact.
131-139: Test for header retrievalThe test
TestBeaconBlockDenebPlus_GetHeaderensures that the header of the block is retrieved correctly and all expected fields match. This test is comprehensive and checks multiple fields, which is good for ensuring the integrity of the header retrieval method.
141-236: Review of marshaling and unmarshaling testsThe
TestBeaconBlockDenebPlus_MarshalSSZ_UnmarshalSSZfunction tests the serialization and deserialization ofBeaconBlockDenebPlus. The test cases include valid blocks, empty blocks, and a scenario with an invalid buffer size. This comprehensive approach is excellent for testing the robustness of the SSZ implementation. However, ensure that the error handling forUnmarshalSSZin the "Invalid Buffer Size" case is adequately tested.
238-251: Test for tree generationThe test
TestBeaconBlockDenebPlus_GetTreechecks the generation of the tree structure and compares the calculated tree root with an expected root. This test is crucial for verifying the correctness of the tree hash function and is well-implemented.
| func generateBeaconBlockDenebPlus() *types.BeaconBlockDenebPlus { | ||
| var byteArray [256]byte | ||
| byteSlice := byteArray[:] | ||
| return &types.BeaconBlockDenebPlus{ | ||
| BeaconBlockHeaderBase: types.BeaconBlockHeaderBase{ | ||
| Slot: 12345, | ||
| ProposerIndex: 67890, | ||
| ParentBlockRoot: [32]byte{ | ||
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, | ||
| 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32}, | ||
| StateRoot: [32]byte{ | ||
| 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, | ||
| 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}, | ||
| }, | ||
| Body: &types.BeaconBlockBodyDenebPlus{ | ||
| BeaconBlockBodyBase: types.BeaconBlockBodyBase{ | ||
| RandaoReveal: [96]byte{ | ||
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, | ||
| 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, | ||
| 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, | ||
| 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, | ||
| 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, | ||
| 88, 89, 90, 91, 92, 93, 94, 95, 96}, | ||
| Eth1Data: &types.Eth1Data{ | ||
| DepositRoot: common.Root{ | ||
| 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, | ||
| 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10, 0x11, 0x12, 0x13, | ||
| 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, | ||
| 0x1E, 0x1F, | ||
| }, | ||
| DepositCount: 12345, | ||
| BlockHash: gethprimitives.ExecutionHash{ | ||
| 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, | ||
| 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10, 0x11, 0x12, 0x13, | ||
| 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, | ||
| 0x1E, 0x1F, | ||
| }, | ||
| }, | ||
| Graffiti: [32]byte{ | ||
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, | ||
| 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, | ||
| }, | ||
| Deposits: []*types.Deposit{}, | ||
| }, | ||
| ExecutionPayload: &types.ExecutableDataDeneb{ | ||
| LogsBloom: byteSlice, | ||
| ExtraData: make([]byte, 0), | ||
| Transactions: make([][]byte, 0), | ||
| Withdrawals: make([]*engineprimitives.Withdrawal, 0), | ||
| }, | ||
| Attestations: []*types.AttestationData{ | ||
| { | ||
| Slot: 12345, | ||
| Index: 1, | ||
| BeaconBlockRoot: [32]byte{ | ||
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, | ||
| 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, | ||
| }, | ||
| }, | ||
| }, | ||
| SlashingInfo: []*types.SlashingInfo{}, | ||
| BlobKzgCommitments: []eip4844.KZGCommitment{}, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Review of helper function generateBeaconBlockDenebPlus()
This function is crucial as it generates a predefined instance of BeaconBlockDenebPlus for testing. The function correctly initializes all necessary fields with mock data, which is a good practice for isolated unit tests. However, using hardcoded values (like slot numbers, proposer index, and byte arrays) can limit the flexibility of tests. Consider parameterizing these values to increase test versatility.
- func generateBeaconBlockDenebPlus() *types.BeaconBlockDenebPlus {
+ func generateBeaconBlockDenebPlus(slot uint64, proposerIndex uint64, parentBlockRoot [32]byte, stateRoot [32]byte) *types.BeaconBlockDenebPlus {
- Slot: 12345,
- ProposerIndex: 67890,
- ParentBlockRoot: [32]byte{1, 2, 3, ..., 32},
- StateRoot: [32]byte{32, 31, 30, ..., 1},
+ Slot: slot,
+ ProposerIndex: proposerIndex,
+ ParentBlockRoot: parentBlockRoot,
+ StateRoot: stateRoot,Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func generateBeaconBlockDenebPlus() *types.BeaconBlockDenebPlus { | |
| var byteArray [256]byte | |
| byteSlice := byteArray[:] | |
| return &types.BeaconBlockDenebPlus{ | |
| BeaconBlockHeaderBase: types.BeaconBlockHeaderBase{ | |
| Slot: 12345, | |
| ProposerIndex: 67890, | |
| ParentBlockRoot: [32]byte{ | |
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, | |
| 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32}, | |
| StateRoot: [32]byte{ | |
| 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, | |
| 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}, | |
| }, | |
| Body: &types.BeaconBlockBodyDenebPlus{ | |
| BeaconBlockBodyBase: types.BeaconBlockBodyBase{ | |
| RandaoReveal: [96]byte{ | |
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, | |
| 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, | |
| 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, | |
| 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, | |
| 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, | |
| 88, 89, 90, 91, 92, 93, 94, 95, 96}, | |
| Eth1Data: &types.Eth1Data{ | |
| DepositRoot: common.Root{ | |
| 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, | |
| 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10, 0x11, 0x12, 0x13, | |
| 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, | |
| 0x1E, 0x1F, | |
| }, | |
| DepositCount: 12345, | |
| BlockHash: gethprimitives.ExecutionHash{ | |
| 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, | |
| 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10, 0x11, 0x12, 0x13, | |
| 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, | |
| 0x1E, 0x1F, | |
| }, | |
| }, | |
| Graffiti: [32]byte{ | |
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, | |
| 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, | |
| }, | |
| Deposits: []*types.Deposit{}, | |
| }, | |
| ExecutionPayload: &types.ExecutableDataDeneb{ | |
| LogsBloom: byteSlice, | |
| ExtraData: make([]byte, 0), | |
| Transactions: make([][]byte, 0), | |
| Withdrawals: make([]*engineprimitives.Withdrawal, 0), | |
| }, | |
| Attestations: []*types.AttestationData{ | |
| { | |
| Slot: 12345, | |
| Index: 1, | |
| BeaconBlockRoot: [32]byte{ | |
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, | |
| 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, | |
| }, | |
| }, | |
| }, | |
| SlashingInfo: []*types.SlashingInfo{}, | |
| BlobKzgCommitments: []eip4844.KZGCommitment{}, | |
| }, | |
| } | |
| } | |
| func generateBeaconBlockDenebPlus(slot uint64, proposerIndex uint64, parentBlockRoot [32]byte, stateRoot [32]byte) *types.BeaconBlockDenebPlus { | |
| var byteArray [256]byte | |
| byteSlice := byteArray[:] | |
| return &types.BeaconBlockDenebPlus{ | |
| BeaconBlockHeaderBase: types.BeaconBlockHeaderBase{ | |
| Slot: slot, | |
| ProposerIndex: proposerIndex, | |
| ParentBlockRoot: parentBlockRoot, | |
| StateRoot: stateRoot, | |
| }, | |
| Body: &types.BeaconBlockBodyDenebPlus{ | |
| BeaconBlockBodyBase: types.BeaconBlockBodyBase{ | |
| RandaoReveal: [96]byte{ | |
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, | |
| 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, | |
| 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, | |
| 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, | |
| 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, | |
| 88, 89, 90, 91, 92, 93, 94, 95, 96}, | |
| Eth1Data: &types.Eth1Data{ | |
| DepositRoot: common.Root{ | |
| 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, | |
| 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10, 0x11, 0x12, 0x13, | |
| 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, | |
| 0x1E, 0x1F, | |
| }, | |
| DepositCount: 12345, | |
| BlockHash: gethprimitives.ExecutionHash{ | |
| 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, | |
| 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10, 0x11, 0x12, 0x13, | |
| 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, | |
| 0x1E, 0x1F, | |
| }, | |
| }, | |
| Graffiti: [32]byte{ | |
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, | |
| 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, | |
| }, | |
| Deposits: []*types.Deposit{}, | |
| }, | |
| ExecutionPayload: &types.ExecutableDataDeneb{ | |
| LogsBloom: byteSlice, | |
| ExtraData: make([]byte, 0), | |
| Transactions: make([][]byte, 0), | |
| Withdrawals: make([]*engineprimitives.Withdrawal, 0), | |
| }, | |
| Attestations: []*types.AttestationData{ | |
| { | |
| Slot: 12345, | |
| Index: 1, | |
| BeaconBlockRoot: [32]byte{ | |
| 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, | |
| 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, | |
| }, | |
| }, | |
| }, | |
| SlashingInfo: []*types.SlashingInfo{}, | |
| BlobKzgCommitments: []eip4844.KZGCommitment{}, | |
| }, | |
| } | |
| } |
Summary by CodeRabbit
BeaconBlockDenebPlustype, including versioning, state root setting, body retrieval, marshaling/unmarshaling, and tree generation.