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

Reject durable nonce transactions not signed by authority #25831

Merged

Conversation

jstarry
Copy link
Member

@jstarry jstarry commented Jun 8, 2022

Problem

A transaction should only be processed as a durable nonce transaction if the used nonce account's authority signed the transaction.

Summary of Changes

  • Add a feature gate for only processing durable nonce transactions that have been signed by the nonce authority

Feature Gate Issue: #25835

@jstarry jstarry added v1.9 feature-gate Pull Request adds or modifies a runtime feature gate labels Jun 8, 2022
@jstarry jstarry force-pushed the feat/reject-unauthorized-nonce-txs branch 2 times, most recently from 14c46fb to eb33803 Compare June 8, 2022 03:12
jackcmay
jackcmay previously approved these changes Jun 8, 2022
sdk/program/src/message/sanitized.rs Outdated Show resolved Hide resolved
runtime/src/bank.rs Outdated Show resolved Hide resolved
sdk/src/nonce_account.rs Show resolved Hide resolved
@mergify mergify bot dismissed jackcmay’s stale review June 8, 2022 16:44

Pull request has been modified.

@jstarry jstarry force-pushed the feat/reject-unauthorized-nonce-txs branch from 791f825 to 95fcae6 Compare June 8, 2022 17:18
Copy link
Contributor

@behzadnouri behzadnouri left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -21,13 +24,13 @@ pub fn create_account(lamports: u64) -> RefCell<AccountSharedData> {
}

// TODO: Consider changing argument from Hash to DurableNonce.
Copy link
Contributor

Choose a reason for hiding this comment

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

this // TODO: ... was not a good idea. Since you are changing this area, maybe remove this as well.

Ok(State::Initialized(ref data)) => hash == &data.blockhash(),
_ => false,
Ok(State::Initialized(data)) => (hash == &data.blockhash()).then(|| data),
_ => None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you are changing this line, might be good to make this _ => ... more explicit, and avoid bugs in the future if State ever has a 3rd variant. Something like:

Ok(State::Initialized(data)) => ...
Ok(State::Uninitialized) => None,
Err(_) => None,

@jstarry jstarry merged commit b2b426d into solana-labs:master Jun 8, 2022
@jstarry jstarry deleted the feat/reject-unauthorized-nonce-txs branch June 8, 2022 19:43
mergify bot pushed a commit that referenced this pull request Jun 8, 2022
(cherry picked from commit b2b426d)

# Conflicts:
#	runtime/src/bank.rs
#	runtime/src/nonce_keyed_account.rs
#	sdk/program/src/message/sanitized.rs
#	sdk/src/feature_set.rs
#	send-transaction-service/src/send_transaction_service.rs
mergify bot pushed a commit that referenced this pull request Jun 8, 2022
(cherry picked from commit b2b426d)

# Conflicts:
#	runtime/src/nonce_keyed_account.rs
mergify bot added a commit that referenced this pull request Jun 8, 2022
…25831) (#25850)

* Reject durable nonce transactions not signed by authority (#25831)

(cherry picked from commit b2b426d)

# Conflicts:
#	runtime/src/nonce_keyed_account.rs

* resolve conflicts

Co-authored-by: Justin Starry <justin@solana.com>
mergify bot added a commit that referenced this pull request Jun 8, 2022
…25831) (#25849)

* Reject durable nonce transactions not signed by authority (#25831)

(cherry picked from commit b2b426d)

# Conflicts:
#	runtime/src/bank.rs
#	runtime/src/nonce_keyed_account.rs
#	sdk/program/src/message/sanitized.rs
#	sdk/src/feature_set.rs
#	send-transaction-service/src/send_transaction_service.rs

* resolve conflicts

Co-authored-by: Justin Starry <justin@solana.com>
@aeyakovenko aeyakovenko mentioned this pull request Jun 9, 2022
11 tasks
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 27, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 27, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 27, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 28, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 28, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 28, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 28, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 28, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 28, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 29, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 29, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-gate Pull Request adds or modifies a runtime feature gate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants