Skip to content

Don't dereference SubstrateBlock on revert#394

Merged
AlexandruCihodaru merged 13 commits intomasterfrom
acihodaru/update-latest
Nov 12, 2025
Merged

Don't dereference SubstrateBlock on revert#394
AlexandruCihodaru merged 13 commits intomasterfrom
acihodaru/update-latest

Conversation

@AlexandruCihodaru
Copy link
Copy Markdown

update_latest took ownership of the Substrateblock and wrapped it in Arc::new. During the rollback/revert operations a caller might try to update the latest block to a block obtained by block_by_number. If one of the "Best" or "Finalized" subscription is lagging this could make block_by_number to return an strong reference. This prevents the caller to update the best block since it will not be able to unwrap the strong reference.

Depends on: paritytech/polkadot-sdk#10252

update_latest took ownership of the Substrateblock and wrapped it
in Arc::new. During the rollback/revert operations a caller might
try to update the latest block to a block obtained by block_by_number.
If one of the "Best" or "Finalized" subscription is lagging this could
make block_by_number to return an strong reference. This prevents
the caller to update the best block since it will not be able to
unwrap the strong reference.

Signed-off-by: Alexandru Cihodaru <alexandru.cihodaru@parity.io>
@AlexandruCihodaru AlexandruCihodaru self-assigned this Nov 10, 2025
@AlexandruCihodaru AlexandruCihodaru marked this pull request as draft November 10, 2025 14:29
Signed-off-by: Alexandru Cihodaru <alexandru.cihodaru@parity.io>
@alindima alindima changed the title Don't derenference SubstrateBlock on revert Don't dereference SubstrateBlock on revert Nov 10, 2025
Signed-off-by: Alexandru Cihodaru <alexandru.cihodaru@parity.io>
Copy link
Copy Markdown

@iulianbarbu iulianbarbu left a comment

Choose a reason for hiding this comment

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

LGTM! Only to update to polkdaot-sdk/master after PG's PR is merged, before merging.

Signed-off-by: Alexandru Cihodaru <alexandru.cihodaru@parity.io>
Signed-off-by: Alexandru Cihodaru <alexandru.cihodaru@parity.io>
@AlexandruCihodaru AlexandruCihodaru marked this pull request as ready for review November 11, 2025 13:51
Signed-off-by: Alexandru Cihodaru <alexandru.cihodaru@parity.io>
This reverts commit e8584dc.
Signed-off-by: Alexandru Cihodaru <alexandru.cihodaru@parity.io>
Signed-off-by: Alexandru Cihodaru <alexandru.cihodaru@parity.io>
Signed-off-by: Alexandru Cihodaru <alexandru.cihodaru@parity.io>
Copy link
Copy Markdown

@alindima alindima left a comment

Choose a reason for hiding this comment

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

Nice catch!

@AlexandruCihodaru AlexandruCihodaru merged commit ecf0ce0 into master Nov 12, 2025
28 of 31 checks passed
@AlexandruCihodaru AlexandruCihodaru deleted the acihodaru/update-latest branch November 12, 2025 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants