fix: TransactionMessage.decompile() now counts the correct number of unsigned, writable accounts

[steveluscher]

Problem

Message headers give you the following:

• number of required signatures (numRequiredSignatures)
• number of read-only signed accounts (numReadonlySignedAccounts)
• number of read-only unsigned accounts (numReadonlyUnsignedAccounts)

If we want to know how many writable unsigned accounts there are, we have to:

• start with the total number of accounts
• subtract the number of those that are signers (numRequiredSignatures)
• subtract the number of read-only unsigned accounts (numReadonlyUnsignedAccounts)

That spells:

const numWritableUnsignedAccounts =
  message.staticAccountKeys.length -
  numRequiredSignatures -
  numReadonlyUnsignedAccounts;

Unfortunately, we forgot to subtract out the number of signers, making it possible to overcount numWritableUnsignedAccounts. This played havoc when deserializing messages like the one in #28900's repro.

All of this could result in accounts being needlessly marked as writeable, which could only serve to make the network slower (ie. causing the validator to run transactions in serial that could otherwise have been safely run in parallel).

Summary of Changes

• Fix the calculation of numWritableUnsignedAccounts
• Write a test that asserts you can decompile a legacy message with TransactionMessage
• Copy the repro from [web3.js] Discrepancy between legacy Transaction API and new VersionedTransaction API #28900 into a test to validate this fix and to serve as a regression going forward.

Fixes #28900.

[codecov]

Codecov Report

Merging #28990 (b08bce0) into master (4267a15) will decrease coverage by 0.4%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #28990     +/-   ##
=========================================
- Coverage    77.1%    76.7%   -0.5%     
=========================================
  Files          55       55             
  Lines        2934     3084    +150     
  Branches      408      454     +46     
=========================================
+ Hits         2264     2367    +103     
- Misses        529      559     +30     
- Partials      141      158     +17     

[steveluscher]

Without this change, the test in this commit incorrectly marks the last account as writable.

[steveluscher]

Got the idea to double check the Rust code, and found it to already be correct.

solana/sdk/program/src/message/versions/v0/mod.rs

Lines 284 to 286 in 98525dd

	let num_unsigned_accounts = num_account_keys.saturating_sub(num_signed_accounts);
	let num_writable_unsigned_accounts = num_unsigned_accounts
	.saturating_sub(usize::from(header.num_readonly_unsigned_accounts));