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

Verify sender account type in mempool #2634

Merged
merged 2 commits into from
Jun 18, 2024
Merged

Verify sender account type in mempool #2634

merged 2 commits into from
Jun 18, 2024

Conversation

styppo
Copy link
Member

@styppo styppo commented Jun 17, 2024

The account type on the sender side is now properly checked in the mempool via reserve_balance().

The intrinsic staking contract account type check is removed entirely from the transaction verification. A mismatch on the recipient side will now result in a failed transaction, which brings it in line with other account type mismatches.

Closes #2605

@sisou
Copy link
Member

sisou commented Jun 17, 2024

Is reserve_balance(), which now has the check, also called by validators that receive the produced block without having seen the tx in their mempool?I.e. does this protect against a malicious validator that includes such a tx in a block?

@styppo
Copy link
Member Author

styppo commented Jun 17, 2024

No, reserve_balance() is only used by the mempool. The sender account type is checked during commit here and here, i.e. a transaction with an incorrect sender account type cannot be included in a block. The mempool now rejects these transactions such that validators don't attempt to produce invalid blocks.

None
}
}

/// The given account must correspond to the sender of the given transaction.
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity: what happens if it isn't?

The account type on the sender side is now properly checked in the mempool via `reserve_balance`. A mismatch on the recipient side will now result in a failed transaction, which brings it in line with other account type mismatches.
@hrxi hrxi mentioned this pull request Jun 18, 2024
@hrxi hrxi merged commit 9e3788c into albatross Jun 18, 2024
6 checks passed
@hrxi hrxi deleted the styppo/mempool branch June 18, 2024 19:27
@jsdanielh jsdanielh added this to the Nimiq PoS Mainnet milestone Jun 22, 2024
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.

Staking transactions are not enforced to be to / from the staking contract
5 participants