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

No runtime check to ensure transaction fee-payer is a debitable account #6339

Closed
CriesofCarrots opened this issue Oct 11, 2019 · 2 comments · Fixed by #6454
Closed

No runtime check to ensure transaction fee-payer is a debitable account #6339

CriesofCarrots opened this issue Oct 11, 2019 · 2 comments · Fixed by #6454
Assignees

Comments

@CriesofCarrots
Copy link
Contributor

Problem

Currently, all credit-debit checks happen inside message_processor::process_message. However, transaction fees get charged upstream in accounts::load_tx_accounts. This means that if a credit-only account is the first signer, it gets successfully charged the fee if it can afford it, potentially adding non-determinism to the chain. 🙀

Proposed Solution

Update runtime to reject transactions pre-fee if the fee payer is not locked as credit-debit

@CriesofCarrots CriesofCarrots self-assigned this Oct 11, 2019
@garious
Copy link
Contributor

garious commented Oct 11, 2019

Can we reject the transaction before any accounts are loaded? Otherwise you can DoS the leader by having it load a bunch of huge accounts without having to pay anything. Before SigVerify or any deserialize() would be ideal.

@CriesofCarrots
Copy link
Contributor Author

Can we reject the transaction before any accounts are loaded? Otherwise you can DoS the leader by having it load a bunch of huge accounts without having to pay anything. Before SigVerify or any deserialize() would be ideal.

Yes, definitely. I am inclined to build off #6236, actually, and put this check in do_get_packet_offsets

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 a pull request may close this issue.

3 participants