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

Fix ConnectTransactionError::MissingTransactionNonce #1500

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

azarovh
Copy link
Member

@azarovh azarovh commented Jan 25, 2024

When we spend from delegation we increment nonces: 1 -> 2 -> 3
When we undo such txs we decrement nonces.

But if we spend the delegation entirely and the pool is decommissioned we delete nonce from db: 1->2->3->None
Now if we want to undo that last transaction we need to read the nonce and decrement it, but it’s not in the db because we deleted it.

The simplest solution would be to not delete nonces from the db.

@azarovh azarovh force-pushed the fix/missing_tx_nonce branch from a4c4e9a to c386d32 Compare January 25, 2024 13:10
Comment on lines +442 to 444
// When deleting a delegation nonce value is preserved in the db
// in case reorg happens so we don't have to restore it
Some(
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure we're not doing anything else like this. We should only delete such information in a pruned database after the max reorg depth.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please make sure we're not doing anything else like this.

I'd merge this and then I'll go thought the code and see what tests I can add.

We should only delete such information in a pruned database after the max reorg depth.

we don't do this yet

Copy link
Contributor

Choose a reason for hiding this comment

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

I know we don't do it. But I mean that such deletions shouldn't happen unless we do it.

@TheQuantumPhysicist
Copy link
Contributor

P.S. This is a draft so that if the solution is acceptable I will have to add fork for testnet.

I don't think you need a fork. This isn't a validation rule.

@azarovh azarovh marked this pull request as ready for review January 25, 2024 14:16
@azarovh azarovh merged commit 562dc5b into master Jan 25, 2024
27 checks passed
@azarovh azarovh deleted the fix/missing_tx_nonce branch January 25, 2024 15:35
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.

2 participants