L2 sync: verify and apply task tests #260
Conversation
Mohiiit
force-pushed
the
test/l2-verify
branch
from
ccc2725
to
d590ccb
Compare
| assert_ne!(result, contracts_trie_root, "State root should not be equal to contracts_trie_root"); | ||
| assert_ne!(result, classes_trie_root, "State root should not be equal to classes_trie_root"); |
There was a problem hiding this comment.
nitpick: is this necessary? considering we're hardcoding 0x6beb971880d4b4996b10fe613b8d49fa3dda8f8b63156c919077e08c534d06e, there's only one way that's possible.
| /// 2. The returned header contains the expected block number. | ||
| #[test] | ||
| fn test_block_hash_success() { | ||
| let block = PreValidatedBlock { |
There was a problem hiding this comment.
can you create_dummy_block by moving it outside the module?
| @@ -28,3 +28,6 @@ bonsai-trie = { workspace = true } | |||
| starknet-core = { workspace = true } | |||
| starknet-types-core = { workspace = true } | |||
| starknet_api = { workspace = true } | |||
|
|
|||
| [dev-dependencies] | |||
| tempfile = "3.10" | |||
There was a problem hiding this comment.
Can we import a tempfile from workspace here ?
Mohiiit
force-pushed
the
test/l2-verify
branch
from
826c85c
to
d73d9a3
Compare
| /// | ||
| /// This function generates an UnverifiedHeader with predefined values, | ||
| /// useful for creating consistent test scenarios. | ||
| fn create_dummy_unverified_header() -> UnverifiedHeader { |
There was a problem hiding this comment.
please include the dummy functions directly in the structure declaration file to enable easy reuse throughout the codebase. For example, the create_dummy_unverified_header function could be modified to take a protocol_version parameter
There was a problem hiding this comment.
when you say structure declaration file, what does it mean? should have these helpers in a different tesl util file?
There was a problem hiding this comment.
for example, I would see this function fitting well in the same file as the UnverifiedHeader struct
There was a problem hiding this comment.
I moved it to another file, wdyt?
| class_hash: Felt::from_hex_unchecked("0xfedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321"), | ||
| }]; | ||
|
|
||
| let replaced_classes = vec
There was a problem hiding this comment.
Seems good to me. I agree with @jbcaron it would be nice to refactor dummy functions into a separate file. I also left a few minor issues to fix, nothing huge. Ping me once you've done the refactor and I will mark this as approved :)
…nt hash and num function
Mohiiit
force-pushed
the
test/l2-verify
branch
from
93dd750
to
71e70ac
Compare
This PR covers the tests for the l2 sync, verify and apply task
Pull Request type
Please add the labels corresponding to the type of changes your PR introduces:
What is the current behavior?
Resolves: #NA
Does this introduce a breaking change?
No