Manage durable nonce stored value in runtime

[t-nelson]

Problem

As per #7443, managing the stored durable nonce value in a program instruction leaves executions resulting in InstructionError open to continuous replay and fee theft until the stored nonce is successfully advanced.

Summary of Changes

Store nonce account of durable nonce TXs that fail with InstructionError
Check nonce premature-usage in runtime
Advance nonce in runtime

Fixes #7443

[codecov]

Codecov Report

Merging #7684 into master will increase coverage by <.1%.
The diff coverage is 99.2%.

@@           Coverage Diff            @@
##           master   #7684     +/-   ##
========================================
+ Coverage    81.7%   81.7%   +<.1%     
========================================
  Files         241     241             
  Lines       50750   50871    +121     
========================================
+ Hits        41502   41609    +107     
- Misses       9248    9262     +14

[t-nelson]

Dropped the removal of instruction nonce advance to regain advancing explicitly with non-durable-nonce TX (fixes CLI integration tests). Probably a good safe guard to keep around in any case. Now I'm considering only doing the runtime advance on InstructionError. wdyt @rob-solana ?

[rob-solana]

I think I've lost the trail of this.

I thought the fix was going to be pay fee + verify the Nonce instruction worked (do both of these during load()). If the Nonce can't advance, don't collect fee, report load failure.

There'd be no store-side changes needed.

[t-nelson]

The fee theft opportunity presents itself if any instruction in the transaction fails. We have to advance the nonce, or not charge fees. Not charging fees opens us up to DoS, so always advancing the nonce seems like the surest path

[rob-solana]

The fee theft opportunity presents itself if any instruction in the transaction fails. We have to advance the nonce, or not charge fees. Not charging fees opens us up to DoS, so always advancing the nonce seems like the surest path

I think we're on the same page there. I'd hoped that running the Nonce instruction or checking that the Nonce instruction would succeed would be part of load()

like, do it here: https://github.com/solana-labs/solana/pull/7684/files#diff-0104dfa96426a3a49428815872a0356eR154

[t-nelson]

The new_nonce != stored_nonce check is. However thinking more on this, without checking for the authority's sig, I'm pretty sure anyone can advance any nonce account now...

Lemme see how gross executing the instruction is

[t-nelson]

This might not actually be too bad! 🤞

[t-nelson]

@rob-solana Sorry, didn't see your edit

like, do it here: https://github.com/solana-labs/solana/pull/7684/files#diff-0104dfa96426a3a49428815872a0356eR154

I initially put the nonce ix pre-execution in the caller, since we had more of the stuff needed to call process_message()-based function. It's in another branch ATM, top four at https://github.com/t-nelson/solana/commits/full-nonce-ix-check

I don't think this'll work purely load-side, though. The problem is the initial TX failing InstructionError having its successful NonceAdvance rolled back, not the replays being loaded. Those willl all get tagged by verify_nonce() and result in a TransactionError::BlockhashNotFound as implemented here. Or am I missing something and the pre-execution is supposed to check something else? I've struggled to get to the pre-execution failure with a test

[rob-solana]

Ok, I think you're right. I think what you have will work.

[t-nelson]

Thanks!