Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pending block and state improvements #480

Merged
merged 23 commits into from
Jun 6, 2024
Merged

Pending block and state improvements #480

merged 23 commits into from
Jun 6, 2024

Conversation

mikiw
Copy link
Contributor

@mikiw mikiw commented Jun 3, 2024

Usage related changes

  • fix of starknet_getNonce on block_id=pending

Development related changes

  • after an answer from Noa is clearer that we should always have a pending block (even if it's empty) and a pending state (even if it's the same as latest state)
  • new logic assumes that we always operate on a pending state and clone it to state once a new block is mined
  • since pending block and state are always there block-on-demand mode is only responsible for block creation

Checklist:

  • Checked out the contribution guidelines
  • Applied formatting - ./scripts/format.sh
  • No linter errors - ./scripts/clippy_check.sh
  • No unused dependencies - ./scripts/check_unused_deps.sh
  • No spelling errors - ./scripts/check_spelling.sh
  • Performed code self-review
  • Rebased to the latest commit of the target branch (or merged it into my branch)
  • Updated the docs if needed - ./website/README.md
  • Linked the issues resolvable by this PR - linking info
  • Updated the tests if needed; all passing - execution info

@mikiw mikiw marked this pull request as draft June 3, 2024 14:24
Mikołaj Woźniak added 2 commits June 4, 2024 15:03
@mikiw mikiw changed the title pending block and state refactor Pending block and state improvements Jun 4, 2024
@mikiw mikiw linked an issue Jun 4, 2024 that may be closed by this pull request
2 tasks
@mikiw
Copy link
Contributor Author

mikiw commented Jun 4, 2024

TODO: #474 is fixed by this PR buy remember to add test for that

@mikiw mikiw marked this pull request as ready for review June 5, 2024 10:46
@mikiw
Copy link
Contributor Author

mikiw commented Jun 5, 2024

I also tried to fix hardcoded nonces in blocks_on_demand_tests but I couldn't.

I tried to use predeployed_account.lock().unwrap().set_block_id(BlockId::Tag(BlockTag::Pending)); but it led to the problem with a clone the method clone exists for struct SingleOwnerAccount<JsonRpcClient<HttpTransport>, LocalWallet>, but its trait bounds were not satisfied.

main...pending-block-improvments-test-check-nonce-bug here is a branch - Alea iacta est if anyone can fix it it would be great, maybe my rust skills are not sufficient.

I would just wait for starknet-rs update and just remove it later.

website/docs/blocks.md Outdated Show resolved Hide resolved
website/docs/blocks.md Outdated Show resolved Hide resolved
crates/starknet-devnet/tests/test_account_selection.rs Outdated Show resolved Hide resolved
crates/starknet-devnet-core/src/starknet/mod.rs Outdated Show resolved Hide resolved
crates/starknet-devnet/tests/test_blocks_on_demand.rs Outdated Show resolved Hide resolved
crates/starknet-devnet/tests/test_account_selection.rs Outdated Show resolved Hide resolved
crates/starknet-devnet/tests/test_blocks_on_demand.rs Outdated Show resolved Hide resolved
crates/starknet-devnet/tests/test_blocks_on_demand.rs Outdated Show resolved Hide resolved
crates/starknet-devnet/tests/test_blocks_on_demand.rs Outdated Show resolved Hide resolved
@mikiw mikiw requested a review from FabijanC June 6, 2024 12:06
Copy link
Contributor

@marioiordanov marioiordanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, because I am not able to understand the state logic

@mikiw mikiw merged commit bab781a into main Jun 6, 2024
1 check passed
@mikiw mikiw deleted the get-nonce-fix branch June 6, 2024 13:11
@FabijanC FabijanC linked an issue Jun 12, 2024 that may be closed by this pull request
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants