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

Check that transaction fee-payer is a debitable account #6454

Merged

Conversation

CriesofCarrots
Copy link
Contributor

@CriesofCarrots CriesofCarrots commented Oct 18, 2019

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.

Summary of Changes

  • Add debitable check of first signer account (fee payer) to sigveryify::do_get_offsets so that it gets executed before any signature verification or transaction deserialization occurs
  • Also updates relevant solana-cli stake and vote transactions to use Transaction::new_signed_with_payer to ensure they don't fail this new processing.

Fixes #6339

@codecov
Copy link

codecov bot commented Oct 19, 2019

Codecov Report

Merging #6454 into master will decrease coverage by 1.1%.
The diff coverage is 58.7%.

@@           Coverage Diff            @@
##           master   #6454     +/-   ##
========================================
- Coverage    79.9%   78.7%   -1.2%     
========================================
  Files         217     217             
  Lines       41340   41974    +634     
========================================
+ Hits        33039   33071     +32     
- Misses       8301    8903    +602

garious
garious previously approved these changes Oct 19, 2019
Copy link
Contributor

@garious garious left a comment

Choose a reason for hiding this comment

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

Great find and patch!

@CriesofCarrots CriesofCarrots force-pushed the fee-payer-debitable-check branch from d6f3844 to d1be927 Compare October 19, 2019 03:10
@mergify mergify bot dismissed garious’s stale review October 19, 2019 03:10

Pull request has been modified.

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label Oct 19, 2019
@solana-grimes solana-grimes merged commit 785c257 into solana-labs:master Oct 19, 2019
@CriesofCarrots CriesofCarrots deleted the fee-payer-debitable-check branch October 21, 2019 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No runtime check to ensure transaction fee-payer is a debitable account
3 participants