v3.1: svm: build NonceInfo during transaction processing (backport of #9455)

[mergify]

Problem

the present nonce flow performs all message and nonce account checks at the runtime level, constructs an advanced nonce account to roll back to in case of loading or execution failure. the nonce account checks are then repeated at the svm level, to allow for multiple transactions which write-lock the nonce account to be executed in the same batch

this is unfortunately unsound. because all rollback nonce states are constructed before any transaction is executed, a nonce account can be mutated in one successful transaction, and then a second failing transaction can roll the nonce account to what it would have been if the first transaction failed

Summary of Changes

we modify the runtime-level nonce checks to only produce the pubkey of the nonce account if validation is successful. svm produces the rollback account state immediately prior to transaction execution, properly accounting for state changes induced by prior transactions

the new svm-level account checks are slightly more strict: we properly validate the nonce signer is contained within the instruction, and we validate that the stored nonce value matches the message recent blockhash. we do this to reduce code duplication, as it lets us use the same verify_nonce_account helper as runtime, instead of reimplementing the logic ad hoc. this change is consensus-safe because no such transaction would pass the equivalent runtime-level checks

in the future it is possible to remove the runtime-level nonce account checks by feature gate, cleanly separating runtime-level message checks from svm-level account checks. such a change is out of scope here

---

This is an automatic backport of pull request #9455 done by Mergify.

[mergify]

Cherry-pick of 346847e has failed:

On branch mergify/bp/v3.1/pr-9455
Your branch is up to date with 'origin/v3.1'.

You are currently cherry-picking commit 346847efe.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   feature-set/src/lib.rs
	modified:   runtime/src/bank.rs
	modified:   runtime/src/bank/tests.rs

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   runtime/src/bank/check_transactions.rs
	both modified:   svm/src/account_loader.rs
	both modified:   svm/src/transaction_processor.rs
	both modified:   svm/tests/integration_test.rs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

[2501babe]

this is unfortunately not clean to resolve by hand, we would need to backport #9155 and #9246 first, which are not related to simd83 but also touch nonce-related code and tests. im in favor of doing this, since a) both of those prs are fairly trivial, and b) mainnet isnt on 3.1 yet so getting the nonce fix into it is more timely than delaying to 4.0

once those prs are in this pr should be conflict-free

@t-nelson do you support doing this?

[t-nelson]

@t-nelson do you support doing this?

probably, i'll look into the proposed prereqs later. do they apply cleanly to v3.1?

how would #9455 need to change to go straight to v3.1?

[t-nelson]

ack. i'm game for #9155 and #9246 in v3.1

[codecov-commenter]

Codecov Report

❌ Patch coverage is 98.87006% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.2%. Comparing base (ae69a00) to head (4ff93f2).
⚠️ Report is 1 commits behind head on v3.1.

Additional details and impacted files

@@            Coverage Diff            @@
##             v3.1    #9756     +/-   ##
=========================================
- Coverage    83.2%    83.2%   -0.1%     
=========================================
  Files         865      865             
  Lines      376108   376101      -7     
=========================================
- Hits       313148   313131     -17     
- Misses      62960    62970     +10     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[2501babe]

1:  346847efe6 ! 1:  4ff93f2c9d svm: build NonceInfo during transaction processing (#9455)
    @@ runtime/src/bank/check_transactions.rs
     (import noise)
    @@ runtime/src/bank/check_transactions.rs
     (import noise)
    @@ svm/src/account_loader.rs: pub(crate) enum TransactionLoadResult {
      }

      #[derive(PartialEq, Eq, Debug, Clone)]
    --#[cfg_attr(feature = "svm-internal", qualifier_attr::field_qualifiers(nonce(pub)))]
    -+#[cfg_attr(
    -+    feature = "svm-internal",
    -+    qualifier_attr::field_qualifiers(nonce_address(pub))
    -+)]
    +-#[cfg_attr(feature = "svm-internal", field_qualifiers(nonce(pub)))]
    ++#[cfg_attr(feature = "svm-internal", field_qualifiers(nonce_address(pub)))]
      pub struct CheckedTransactionDetails {
     -    pub(crate) nonce: Option<NonceInfo>,
     +    pub(crate) nonce_address: Option<Pubkey>,
    @@ svm/src/transaction_processor.rs: impl<FG: ForkGraph> TransactionBatchProcessor<
     -            })
     -            .and_then(
     -                |current_nonce_versions| match current_nonce_versions.state() {
    --                    NonceState::Initialized(current_nonce_data) => {
    +-                    NonceState::Initialized(ref current_nonce_data) => {
     -                        let nonce_can_be_advanced =
     -                            &current_nonce_data.durable_nonce != next_durable_nonce;
     -
    @@ svm/src/transaction_processor.rs: mod tests {
     (testcase noise)

i had to resolve conflicts by hand to avoid backporting #9271. diffing master vs v3.1 svm/ files directly isnt readable because there are a few other large unrelated changes in master, but to aid in review the above range-diff shows that the original commit and this cherrypick should be identical

[t-nelson]

lgtm. we're running this one (and the two dependents) past @nbelenkov and ar for extra confidence. no need to hold merge imo

[nbelenkov]

This looks good to me, will send it ar way

[t-nelson]

who needs to sme here? we want this in before tomorrow so it lands in the mb upgrade candidate

[2501babe]

i added @jstarry because he reviewed the original pr but anyone can approve, it just needs "two, at least one of whom is on backports commitee." ill ping him but @nbelenkov can probably also be second reviewer

[nbelenkov]

starry is a better sme to approve, but I am also happy to approve if needed