feat(state): Implements reconsider_block method #9260

elijahhampton · 2025-02-17T02:35:19Z

Motivation

The changes create and test a reconsider_block method to later be integrated into the zebra RPC. This method reconsiders previously invalidated_blocks into a chain that exist in the chain_set. Closes #8842.

Specifications & References

Tracking: Support invalidating and reconsidering blocks in the NonFinalizedState via RPC methods #8634
Add a reconsider_block() method to the NonFinalizedState for validating/committing a previously invalidated block and its invalidated descendents #8842

Solution

Added a reconsider_block method to NonFinalizedState that allows previously invalidated blocks to be reconsidered and re-inserted into the chain.

Tests

Added test coverage in 'zebra/zebra-state/src/service/non_finalized_state/tests/vectors.rs'

Follow-up Work

PR Author's Checklist

The PR name will make sense to users.
The PR provides a CHANGELOG summary.
The solution is tested.
The documentation is up to date.
The PR has a priority label.

PR Reviewer's Checklist

The PR Author's checklist is complete.
The PR resolves the issue.

arya2

Thank you! This looks good, I left a suggestion for some cleanup, but there are no blockers to merging this as-is if preferred.

zebra-state/src/service/non_finalized_state.rs

zebra-state/src/error.rs

mergify · 2025-02-19T22:01:02Z

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #9260 has been dequeued, merge conditions unmatch:

#approved-reviews-by >= 1 [🛡 GitHub branch protection]
#approved-reviews-by >= 1 [🛡 GitHub repository ruleset rule]
any of [🛡 GitHub branch protection]:
- check-neutral = Test stable on ubuntu-latest
- check-skipped = Test stable on ubuntu-latest
- check-success = Test stable on ubuntu-latest
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Install zebrad from lockfile without cache on ubuntu-latest
- check-skipped = Install zebrad from lockfile without cache on ubuntu-latest
- check-success = Install zebrad from lockfile without cache on ubuntu-latest
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build tower-batch-control crate
- check-skipped = Build tower-batch-control crate
- check-success = Build tower-batch-control crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-chain crate
- check-skipped = Build zebra-chain crate
- check-success = Build zebra-chain crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-consensus crate
- check-skipped = Build zebra-consensus crate
- check-success = Build zebra-consensus crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-grpc crate
- check-skipped = Build zebra-grpc crate
- check-success = Build zebra-grpc crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-network crate
- check-skipped = Build zebra-network crate
- check-success = Build zebra-network crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-node-services crate
- check-skipped = Build zebra-node-services crate
- check-success = Build zebra-node-services crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-rpc crate
- check-skipped = Build zebra-rpc crate
- check-success = Build zebra-rpc crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-scan crate
- check-skipped = Build zebra-scan crate
- check-success = Build zebra-scan crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-script crate
- check-skipped = Build zebra-script crate
- check-success = Build zebra-script crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build tower-fallback crate
- check-skipped = Build tower-fallback crate
- check-success = Build tower-fallback crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-state crate
- check-skipped = Build zebra-state crate
- check-success = Build zebra-state crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build zebra-utils crate
- check-skipped = Build zebra-utils crate
- check-success = Build zebra-utils crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Test beta on ubuntu-latest
- check-skipped = Test beta on ubuntu-latest
- check-success = Test beta on ubuntu-latest
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Test stable on ubuntu-latest
- check-skipped = Test stable on ubuntu-latest
- check-success = Test stable on ubuntu-latest
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Build CI Docker / Build images
- check-skipped = Build CI Docker / Build images
- check-success = Build CI Docker / Build images
#changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
#changes-requested-reviews-by = 0 [🛡 GitHub repository ruleset rule]
#review-threads-unresolved = 0 [🛡 GitHub repository ruleset rule]
any of [🛡 GitHub branch protection]:
- check-skipped = Test CD custom Docker config file / Test custom-conf in Docker
- check-neutral = Test CD custom Docker config file / Test custom-conf in Docker
- check-success = Test CD custom Docker config file / Test custom-conf in Docker
any of [🛡 GitHub repository ruleset rule]:
- check-success = Rustfmt
- check-neutral = Rustfmt
- check-skipped = Rustfmt
any of [🛡 GitHub repository ruleset rule]:
- check-success = mergefreeze
- check-neutral = mergefreeze
- check-skipped = mergefreeze
any of [🛡 GitHub repository ruleset rule]:
- check-success = Clippy
- check-neutral = Clippy
- check-skipped = Clippy
any of [🛡 GitHub repository ruleset rule]:
- check-success = Check deny.toml bans
- check-neutral = Check deny.toml bans
- check-skipped = Check deny.toml bans
any of [🛡 GitHub repository ruleset rule]:
- check-success = Check deny.toml bans --all-features
- check-neutral = Check deny.toml bans --all-features
- check-skipped = Check deny.toml bans --all-features
any of [🛡 GitHub repository ruleset rule]:
- check-success = Check deny.toml bans --features default-release-binaries
- check-neutral = Check deny.toml bans --features default-release-binaries
- check-skipped = Check deny.toml bans --features default-release-binaries
any of [🛡 GitHub repository ruleset rule]:
- check-success = Check deny.toml sources
- check-neutral = Check deny.toml sources
- check-skipped = Check deny.toml sources
any of [🛡 GitHub repository ruleset rule]:
- check-success = Check deny.toml sources --all-features
- check-neutral = Check deny.toml sources --all-features
- check-skipped = Check deny.toml sources --all-features
any of [🛡 GitHub repository ruleset rule]:
- check-success = Check deny.toml sources --features default-release-binaries
- check-neutral = Check deny.toml sources --features default-release-binaries
- check-skipped = Check deny.toml sources --features default-release-binaries
any of [🛡 GitHub repository ruleset rule]:
- check-success = Check Cargo.lock is up to date
- check-neutral = Check Cargo.lock is up to date
- check-skipped = Check Cargo.lock is up to date
any of [🛡 GitHub repository ruleset rule]:
- check-success = Build zebra-test crate
- check-neutral = Build zebra-test crate
- check-skipped = Build zebra-test crate
any of [🛡 GitHub repository ruleset rule]:
- check-neutral = Mergify Merge Protections
- check-skipped = Mergify Merge Protections
- check-success = Mergify Merge Protections
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Build CD Docker / Build images
- check-neutral = Build CD Docker / Build images
- check-success = Build CD Docker / Build images
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / Check if cached state disks exist for Mainnet / Get Mainnet cached disk
- check-neutral = Integration tests / Check if cached state disks exist for Mainnet / Get Mainnet cached disk
- check-success = Integration tests / Check if cached state disks exist for Mainnet / Get Mainnet cached disk
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / Zebra checkpoint update / Run sync-past-checkpoint test
- check-neutral = Integration tests / Zebra checkpoint update / Run sync-past-checkpoint test
- check-success = Integration tests / Zebra checkpoint update / Run sync-past-checkpoint test
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / Zebra tip update / Run update-to-tip test
- check-neutral = Integration tests / Zebra tip update / Run update-to-tip test
- check-success = Integration tests / Zebra tip update / Run update-to-tip test
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / Generate checkpoints mainnet / Run checkpoints-mainnet test
- check-neutral = Integration tests / Generate checkpoints mainnet / Run checkpoints-mainnet test
- check-success = Integration tests / Generate checkpoints mainnet / Run checkpoints-mainnet test
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / Zebra tip JSON-RPC / Run fully-synced-rpc test
- check-neutral = Integration tests / Zebra tip JSON-RPC / Run fully-synced-rpc test
- check-success = Integration tests / Zebra tip JSON-RPC / Run fully-synced-rpc test
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / lightwalletd tip send / Run lwd-send-transactions test
- check-neutral = Integration tests / lightwalletd tip send / Run lwd-send-transactions test
- check-success = Integration tests / lightwalletd tip send / Run lwd-send-transactions test
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / get block template / Run get-block-template test
- check-neutral = Integration tests / get block template / Run get-block-template test
- check-success = Integration tests / get block template / Run get-block-template test
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / submit block / Run submit-block test
- check-neutral = Integration tests / submit block / Run submit-block test
- check-success = Integration tests / submit block / Run submit-block test
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / lightwalletd tip update / Run lwd-update-sync test
- check-neutral = Integration tests / lightwalletd tip update / Run lwd-update-sync test
- check-success = Integration tests / lightwalletd tip update / Run lwd-update-sync test
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Integration tests / lightwalletd GRPC tests / Run lwd-grpc-wallet test
- check-neutral = Integration tests / lightwalletd GRPC tests / Run lwd-grpc-wallet test
- check-success = Integration tests / lightwalletd GRPC tests / Run lwd-grpc-wallet test
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Unit tests / Test all
- check-neutral = Unit tests / Test all
- check-success = Unit tests / Test all
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Unit tests / Test with fake activation heights
- check-neutral = Unit tests / Test with fake activation heights
- check-success = Unit tests / Test with fake activation heights
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Unit tests / Test checkpoint sync from empty state
- check-neutral = Unit tests / Test checkpoint sync from empty state
- check-success = Unit tests / Test checkpoint sync from empty state
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Unit tests / Test integration with lightwalletd
- check-neutral = Unit tests / Test integration with lightwalletd
- check-success = Unit tests / Test integration with lightwalletd
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Unit tests / Test CI default Docker config file / Test default-conf in Docker
- check-neutral = Unit tests / Test CI default Docker config file / Test default-conf in Docker
- check-success = Unit tests / Test CI default Docker config file / Test default-conf in Docker
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Unit tests / Test CI testnet Docker config file / Test testnet-conf in Docker
- check-neutral = Unit tests / Test CI testnet Docker config file / Test testnet-conf in Docker
- check-success = Unit tests / Test CI testnet Docker config file / Test testnet-conf in Docker
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Unit tests / Test CI custom Docker config file / Test custom-conf in Docker
- check-neutral = Unit tests / Test CI custom Docker config file / Test custom-conf in Docker
- check-success = Unit tests / Test CI custom Docker config file / Test custom-conf in Docker
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Test CD default Docker config file / Test default-conf in Docker
- check-neutral = Test CD default Docker config file / Test default-conf in Docker
- check-success = Test CD default Docker config file / Test default-conf in Docker
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Test CD testnet Docker config file / Test testnet-conf in Docker
- check-neutral = Test CD testnet Docker config file / Test testnet-conf in Docker
- check-success = Test CD testnet Docker config file / Test testnet-conf in Docker
any of [🛡 GitHub repository ruleset rule]:
- check-skipped = Test CD custom Docker config file / Test custom-conf in Docker
- check-neutral = Test CD custom Docker config file / Test custom-conf in Docker
- check-success = Test CD custom Docker config file / Test custom-conf in Docker
any of [🛡 GitHub repository ruleset rule]:
- check-success = Get disk name / Get Mainnet cached disk
- check-neutral = Get disk name / Get Mainnet cached disk
- check-skipped = Get disk name / Get Mainnet cached disk
any of [🛡 GitHub repository ruleset rule]:
- check-success = mergefreeze
- check-neutral = mergefreeze
- check-skipped = mergefreeze

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

zebra-state/src/constants.rs

arya2

This is looking great, thank you for adding the IndexMap.

There's one small issue around adding chains to the non-finalized state with roots below the finalized tip, but otherwise this PR looks ready to merge.

zebra-state/src/service/non_finalized_state.rs

zebra-state/src/constants.rs

zebra-state/src/service/non_finalized_state.rs

zebra-state/src/error.rs

zebra-state/src/service/non_finalized_state.rs

arya2

Thank you!! 🎉 🎉 🎉

arya2 · 2025-02-25T19:39:49Z

@Mergifyio update

mergify · 2025-02-25T19:39:53Z

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/README.md without workflows permission

arya2 · 2025-02-25T20:04:28Z

@elijahhampton could you do a rebase off main?

elijahhampton · 2025-02-25T20:05:41Z

@elijahhampton could you do a rebase off main?

Yes

arya2 · 2025-02-25T21:31:24Z

@elijahhampton the order of commits and the overall diff still don't look right, let me know if you'd like some help with the rebase via direct commits to this branch or on a fork.

elijahhampton · 2025-02-25T21:32:10Z

@elijahhampton the order of commits and the overall diff still don't look right, let me know if you'd like some help with the rebase via direct commits to this branch or on a fork.

@arya2 Hey, if you don't mind, please and thank you.

…m validate_and_commit if a candidate block's hash is in the map of invalidated blocks. Stores invalidated_blocks by height and clears when finalizing. Checks against non finalized tip hash to create a new chain if parnt_chain doesn't exist. Renames ReconsiderError variant NonPreviouslyInvalidatedBlock to MissingInvalidatedBlock.

…). Removes unused ReconsiderError variant. Opts to refuse block consideration if parent_chain does not exist. Adds db handle to reconsider_block function. Edits max blocks constant documentation

…ized blocks only before checking parent_chain.

arya2 · 2025-02-26T01:46:16Z

@Mergifyio refresh

mergify · 2025-02-26T01:46:31Z

refresh

✅ Pull request refreshed

mpguerra · 2025-02-26T08:34:12Z

@mergify refresh

mergify · 2025-02-26T08:34:16Z

refresh

✅ Pull request refreshed

mergify · 2025-02-26T12:49:04Z

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

elijahhampton requested a review from a team as a code owner February 17, 2025 02:35

elijahhampton requested review from arya2 and removed request for a team February 17, 2025 02:35

github-actions bot added the C-feature Category: New features label Feb 17, 2025

arya2 assigned elijahhampton Feb 17, 2025

arya2 added A-state Area: State / database changes P-Medium ⚡ labels Feb 17, 2025

arya2 previously approved these changes Feb 17, 2025

View reviewed changes

zebra-state/src/service/non_finalized_state.rs Outdated Show resolved Hide resolved

arya2 reviewed Feb 17, 2025

View reviewed changes

zebra-state/src/error.rs Outdated Show resolved Hide resolved

arya2 mentioned this pull request Feb 18, 2025

feat(rpc): Add invalidateblock and reconsiderblock RPC methods #9266

Open

7 tasks

mergify bot added a commit that referenced this pull request Feb 19, 2025

Merge of #9260

321dc6f

mergify bot mentioned this pull request Feb 19, 2025

merge queue: embarking main (0b8bef3) and #9260 together #9278

Closed

elijahhampton dismissed arya2’s stale review via 9765798 February 19, 2025 22:00

elijahhampton requested a review from arya2 February 19, 2025 22:00

elijahhampton commented Feb 19, 2025

View reviewed changes

zebra-state/src/constants.rs Outdated Show resolved Hide resolved

arya2 reviewed Feb 20, 2025

View reviewed changes

zebra-state/src/service/non_finalized_state.rs Outdated Show resolved Hide resolved

zebra-state/src/constants.rs Outdated Show resolved Hide resolved

zebra-state/src/service/non_finalized_state.rs Outdated Show resolved Hide resolved

zebra-state/src/error.rs Outdated Show resolved Hide resolved

elijahhampton requested a review from arya2 February 21, 2025 18:38

arya2 reviewed Feb 21, 2025

View reviewed changes

zebra-state/src/service/non_finalized_state.rs Show resolved Hide resolved

elijahhampton requested a review from arya2 February 22, 2025 01:51

arya2 approved these changes Feb 25, 2025

View reviewed changes

elijahhampton force-pushed the feat(8842)-add-reconsider-block-method branch from b6b119d to b6b0474 Compare February 25, 2025 21:14

elijahhampton requested review from a team as code owners February 25, 2025 21:14

elijahhampton requested review from oxarbitrage and removed request for a team February 25, 2025 21:14

arya2 previously approved these changes Feb 25, 2025

View reviewed changes

elijahhampton added 5 commits February 25, 2025 16:55

Adds reconsider_block method and appropriate test cases

86026aa

Moves MAX_INVALIDATED_BLOCKS constant to constants.rs

0253460

Maintains invalidate_blocks cleanup in reconsider_block and finalize(…

52fc9d9

…). Removes unused ReconsiderError variant. Opts to refuse block consideration if parent_chain does not exist. Adds db handle to reconsider_block function. Edits max blocks constant documentation

Checks the finalized state first to create a new chain from non final…

d71f7ec

…ized blocks only before checking parent_chain.

arya2 dismissed their stale review via d71f7ec February 25, 2025 21:56

arya2 force-pushed the feat(8842)-add-reconsider-block-method branch from b6b0474 to d71f7ec Compare February 25, 2025 21:56

arya2 approved these changes Feb 25, 2025

View reviewed changes

conradoplg approved these changes Feb 25, 2025

View reviewed changes

mergify bot added a commit that referenced this pull request Feb 26, 2025

Merge of #9260

00fa3b4

mergify bot mentioned this pull request Feb 26, 2025

merge queue: embarking main (29ed501) and #9260 together #9303

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(state): Implements reconsider_block method #9260

feat(state): Implements reconsider_block method #9260

elijahhampton commented Feb 17, 2025

arya2 left a comment

mergify bot commented Feb 19, 2025

arya2 left a comment •

edited

Loading

arya2 left a comment

arya2 commented Feb 25, 2025

mergify bot commented Feb 25, 2025

arya2 commented Feb 25, 2025

elijahhampton commented Feb 25, 2025

arya2 commented Feb 25, 2025

elijahhampton commented Feb 25, 2025

arya2 commented Feb 26, 2025

mergify bot commented Feb 26, 2025

mpguerra commented Feb 26, 2025

mergify bot commented Feb 26, 2025

mergify bot commented Feb 26, 2025

feat(state): Implements reconsider_block method #9260

Are you sure you want to change the base?

feat(state): Implements reconsider_block method #9260

Conversation

elijahhampton commented Feb 17, 2025

Motivation

Specifications & References

Solution

Tests

Follow-up Work

PR Author's Checklist

PR Reviewer's Checklist

arya2 left a comment

Choose a reason for hiding this comment

mergify bot commented Feb 19, 2025

arya2 left a comment • edited Loading

Choose a reason for hiding this comment

arya2 left a comment

Choose a reason for hiding this comment

arya2 commented Feb 25, 2025

mergify bot commented Feb 25, 2025

❌ Mergify doesn't have permission to update

arya2 commented Feb 25, 2025

elijahhampton commented Feb 25, 2025

arya2 commented Feb 25, 2025

elijahhampton commented Feb 25, 2025

arya2 commented Feb 26, 2025

mergify bot commented Feb 26, 2025

✅ Pull request refreshed

mpguerra commented Feb 26, 2025

mergify bot commented Feb 26, 2025

✅ Pull request refreshed

mergify bot commented Feb 26, 2025

arya2 left a comment •

edited

Loading