From 2f193266e094fbcb7504e28572c6e06f8871dd26 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Mon, 12 Aug 2024 09:11:30 -0500 Subject: [PATCH 1/5] AccountLocks: validate_account_locks (#2448) --- accounts-db/src/account_locks.rs | 142 +++++++++++++++++++++++- accounts-db/src/accounts.rs | 13 +-- sdk/program/src/message/account_keys.rs | 5 + 3 files changed, 149 insertions(+), 11 deletions(-) diff --git a/accounts-db/src/account_locks.rs b/accounts-db/src/account_locks.rs index dcd512e7d72984..fc2fba7292b10e 100644 --- a/accounts-db/src/account_locks.rs +++ b/accounts-db/src/account_locks.rs @@ -2,8 +2,12 @@ use qualifier_attr::qualifiers; use { ahash::{AHashMap, AHashSet}, - solana_sdk::{pubkey::Pubkey, transaction::TransactionError}, - std::collections::hash_map, + solana_sdk::{ + message::AccountKeys, + pubkey::Pubkey, + transaction::{TransactionError, MAX_TX_ACCOUNT_LOCKS}, + }, + std::{cell::RefCell, collections::hash_map}, }; #[derive(Debug, Default)] @@ -110,9 +114,51 @@ impl AccountLocks { } } +/// Validate account locks before locking. +pub fn validate_account_locks( + account_keys: AccountKeys, + tx_account_lock_limit: usize, +) -> Result<(), TransactionError> { + if account_keys.len() > tx_account_lock_limit { + Err(TransactionError::TooManyAccountLocks) + } else if has_duplicates(account_keys) { + Err(TransactionError::AccountLoadedTwice) + } else { + Ok(()) + } +} + +thread_local! { + static HAS_DUPLICATES_SET: RefCell> = RefCell::new(AHashSet::with_capacity(MAX_TX_ACCOUNT_LOCKS)); +} + +/// Check for duplicate account keys. +fn has_duplicates(account_keys: AccountKeys) -> bool { + // Benchmarking has shown that for sets of 32 or more keys, it is faster to + // use a HashSet to check for duplicates. + // For smaller sets a brute-force O(n^2) check seems to be faster. + const USE_ACCOUNT_LOCK_SET_SIZE: usize = 32; + if account_keys.len() >= USE_ACCOUNT_LOCK_SET_SIZE { + HAS_DUPLICATES_SET.with_borrow_mut(|set| { + let has_duplicates = account_keys.iter().any(|key| !set.insert(*key)); + set.clear(); + has_duplicates + }) + } else { + for (idx, key) in account_keys.iter().enumerate() { + for jdx in idx + 1..account_keys.len() { + if key == &account_keys[jdx] { + return true; + } + } + } + false + } +} + #[cfg(test)] mod tests { - use super::*; + use {super::*, solana_sdk::message::v0::LoadedAddresses}; #[test] fn test_account_locks() { @@ -152,4 +198,94 @@ mod tests { account_locks.unlock_accounts([(&key2, false)].into_iter()); assert!(!account_locks.is_locked_readonly(&key2)); } + + #[test] + fn test_validate_account_locks_valid_no_dynamic() { + let static_keys = &[Pubkey::new_unique(), Pubkey::new_unique()]; + let account_keys = AccountKeys::new(static_keys, None); + assert!(validate_account_locks(account_keys, MAX_TX_ACCOUNT_LOCKS).is_ok()); + } + + #[test] + fn test_validate_account_locks_too_many_no_dynamic() { + let static_keys = &[Pubkey::new_unique(), Pubkey::new_unique()]; + let account_keys = AccountKeys::new(static_keys, None); + assert_eq!( + validate_account_locks(account_keys, 1), + Err(TransactionError::TooManyAccountLocks) + ); + } + + #[test] + fn test_validate_account_locks_duplicate_no_dynamic() { + let duplicate_key = Pubkey::new_unique(); + let static_keys = &[duplicate_key, Pubkey::new_unique(), duplicate_key]; + let account_keys = AccountKeys::new(static_keys, None); + assert_eq!( + validate_account_locks(account_keys, MAX_TX_ACCOUNT_LOCKS), + Err(TransactionError::AccountLoadedTwice) + ); + } + + #[test] + fn test_validate_account_locks_valid_dynamic() { + let static_keys = &[Pubkey::new_unique(), Pubkey::new_unique()]; + let dynamic_keys = LoadedAddresses { + writable: vec![Pubkey::new_unique()], + readonly: vec![Pubkey::new_unique()], + }; + let account_keys = AccountKeys::new(static_keys, Some(&dynamic_keys)); + assert!(validate_account_locks(account_keys, MAX_TX_ACCOUNT_LOCKS).is_ok()); + } + + #[test] + fn test_validate_account_locks_too_many_dynamic() { + let static_keys = &[Pubkey::new_unique()]; + let dynamic_keys = LoadedAddresses { + writable: vec![Pubkey::new_unique()], + readonly: vec![Pubkey::new_unique()], + }; + let account_keys = AccountKeys::new(static_keys, Some(&dynamic_keys)); + assert_eq!( + validate_account_locks(account_keys, 2), + Err(TransactionError::TooManyAccountLocks) + ); + } + + #[test] + fn test_validate_account_locks_duplicate_dynamic() { + let duplicate_key = Pubkey::new_unique(); + let static_keys = &[duplicate_key]; + let dynamic_keys = LoadedAddresses { + writable: vec![Pubkey::new_unique()], + readonly: vec![duplicate_key], + }; + let account_keys = AccountKeys::new(static_keys, Some(&dynamic_keys)); + assert_eq!( + validate_account_locks(account_keys, MAX_TX_ACCOUNT_LOCKS), + Err(TransactionError::AccountLoadedTwice) + ); + } + + #[test] + fn test_has_duplicates_small() { + let mut keys = (0..16).map(|_| Pubkey::new_unique()).collect::>(); + let account_keys = AccountKeys::new(&keys, None); + assert!(!has_duplicates(account_keys)); + + keys[14] = keys[3]; // Duplicate key + let account_keys = AccountKeys::new(&keys, None); + assert!(has_duplicates(account_keys)); + } + + #[test] + fn test_has_duplicates_large() { + let mut keys = (0..64).map(|_| Pubkey::new_unique()).collect::>(); + let account_keys = AccountKeys::new(&keys, None); + assert!(!has_duplicates(account_keys)); + + keys[47] = keys[3]; // Duplicate key + let account_keys = AccountKeys::new(&keys, None); + assert!(has_duplicates(account_keys)); + } } diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index d781bb45e11b8a..9033ceea6e6da5 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -1,6 +1,6 @@ use { crate::{ - account_locks::AccountLocks, + account_locks::{validate_account_locks, AccountLocks}, accounts_db::{ AccountStorageEntry, AccountsAddRootTiming, AccountsDb, LoadHint, LoadedAccount, ScanAccountStorageData, ScanStorageResult, VerifyAccountsHashAndLamportsConfig, @@ -520,7 +520,7 @@ impl Accounts { // Validate the account locks, then get iterator if successful validation. let tx_account_locks_results: Vec> = txs .map(|tx| { - SanitizedTransaction::validate_account_locks(tx.message(), tx_account_lock_limit) + validate_account_locks(tx.account_keys(), tx_account_lock_limit) .map(|_| TransactionAccountLocksIterator::new(tx)) }) .collect(); @@ -530,7 +530,7 @@ impl Accounts { #[must_use] pub fn lock_accounts_with_results<'a>( &self, - txs: impl Iterator, + txs: impl Iterator, results: impl Iterator>, tx_account_lock_limit: usize, ) -> Vec> { @@ -538,11 +538,8 @@ impl Accounts { let tx_account_locks_results: Vec> = txs .zip(results) .map(|(tx, result)| match result { - Ok(()) => SanitizedTransaction::validate_account_locks( - tx.message(), - tx_account_lock_limit, - ) - .map(|_| TransactionAccountLocksIterator::new(tx)), + Ok(()) => validate_account_locks(tx.account_keys(), tx_account_lock_limit) + .map(|_| TransactionAccountLocksIterator::new(tx)), Err(err) => Err(err), }) .collect(); diff --git a/sdk/program/src/message/account_keys.rs b/sdk/program/src/message/account_keys.rs index e7bb569d03643b..e7dbd061cede76 100644 --- a/sdk/program/src/message/account_keys.rs +++ b/sdk/program/src/message/account_keys.rs @@ -17,6 +17,7 @@ pub struct AccountKeys<'a> { impl Index for AccountKeys<'_> { type Output = Pubkey; + #[inline] fn index(&self, index: usize) -> &Self::Output { self.get(index).expect("index is invalid") } @@ -33,6 +34,7 @@ impl<'a> AccountKeys<'a> { /// Returns an iterator of account key segments. The ordering of segments /// affects how account indexes from compiled instructions are resolved and /// so should not be changed. + #[inline] fn key_segment_iter(&self) -> impl Iterator + Clone { if let Some(dynamic_keys) = self.dynamic_keys { [ @@ -51,6 +53,7 @@ impl<'a> AccountKeys<'a> { /// message account keys constructed from static keys, followed by dynamically /// loaded writable addresses, and lastly the list of dynamically loaded /// readonly addresses. + #[inline] pub fn get(&self, mut index: usize) -> Option<&'a Pubkey> { for key_segment in self.key_segment_iter() { if index < key_segment.len() { @@ -63,6 +66,7 @@ impl<'a> AccountKeys<'a> { } /// Returns the total length of loaded accounts for a message + #[inline] pub fn len(&self) -> usize { let mut len = 0usize; for key_segment in self.key_segment_iter() { @@ -77,6 +81,7 @@ impl<'a> AccountKeys<'a> { } /// Iterator for the addresses of the loaded accounts for a message + #[inline] pub fn iter(&self) -> impl Iterator + Clone { self.key_segment_iter().flatten() } From 8223dfc1132869187e45df338b884655d50fd678 Mon Sep 17 00:00:00 2001 From: Yihau Chen Date: Mon, 12 Aug 2024 22:17:23 +0800 Subject: [PATCH 2/5] fix: dependency_on_unit_never_type_fallback (#2551) --- client/src/send_and_confirm_transactions_in_parallel.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client/src/send_and_confirm_transactions_in_parallel.rs b/client/src/send_and_confirm_transactions_in_parallel.rs index aa65ef42a9d93e..571e3566a4782a 100644 --- a/client/src/send_and_confirm_transactions_in_parallel.rs +++ b/client/src/send_and_confirm_transactions_in_parallel.rs @@ -309,7 +309,10 @@ async fn sign_all_messages_and_send( }); } // collect to convert Vec> to Result> - join_all(futures).await.into_iter().collect::>()?; + join_all(futures) + .await + .into_iter() + .collect::>>()?; Ok(()) } From 23529db19def247c047da3006e95b5a3507036c8 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 12 Aug 2024 22:17:51 +0800 Subject: [PATCH 3/5] build(deps): bump serde_json from 1.0.122 to 1.0.124 (#2548) * build(deps): bump serde_json from 1.0.122 to 1.0.124 Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.122 to 1.0.124. - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](https://github.com/serde-rs/json/compare/v1.0.122...v1.0.124) --- updated-dependencies: - dependency-name: serde_json dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] * Update all Cargo files --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- programs/sbf/Cargo.lock | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4bd42822c86cf6..472d0d59806ccc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5045,9 +5045,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.122" +version = "1.0.124" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "784b6203951c57ff748476b126ccb5e8e2959a5c19e5c617ab1956be3dbc68da" +checksum = "66ad62847a56b3dba58cc891acd13884b9c61138d330c0d7b6181713d4fce38d" dependencies = [ "itoa", "memchr", diff --git a/Cargo.toml b/Cargo.toml index 5cc529ea29ab86..2e4e857533db0d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -322,7 +322,7 @@ seqlock = "0.2.0" serde = "1.0.205" # must match the serde_derive version, see https://github.com/serde-rs/serde/issues/2584#issuecomment-1685252251 serde_bytes = "0.11.15" serde_derive = "1.0.205" # must match the serde version, see https://github.com/serde-rs/serde/issues/2584#issuecomment-1685252251 -serde_json = "1.0.122" +serde_json = "1.0.124" serde_with = { version = "2.3.3", default-features = false } serde_yaml = "0.9.34" serial_test = "2.0.0" diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index b340cf6da24faa..c08a7d45d0302b 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -4199,9 +4199,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.122" +version = "1.0.124" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "784b6203951c57ff748476b126ccb5e8e2959a5c19e5c617ab1956be3dbc68da" +checksum = "66ad62847a56b3dba58cc891acd13884b9c61138d330c0d7b6181713d4fce38d" dependencies = [ "itoa", "memchr", From f46d3408b255764ff5e413bb27714c1cea630614 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 12 Aug 2024 22:18:15 +0800 Subject: [PATCH 4/5] build(deps): bump syn from 2.0.72 to 2.0.74 (#2549) Bumps [syn](https://github.com/dtolnay/syn) from 2.0.72 to 2.0.74. - [Release notes](https://github.com/dtolnay/syn/releases) - [Commits](https://github.com/dtolnay/syn/compare/2.0.72...2.0.74) --- updated-dependencies: - dependency-name: syn dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 68 +++++++++++++++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 472d0d59806ccc..6702d3c94e08cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -722,7 +722,7 @@ checksum = "6e0c28dcc82d7c8ead5cb13beb15405b57b8546e93215673ff8ca0349a028107" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -875,7 +875,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -1032,7 +1032,7 @@ dependencies = [ "proc-macro-crate 3.1.0", "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", "syn_derive", ] @@ -1164,7 +1164,7 @@ checksum = "1ee891b04274a59bd38b412188e24b849617b2e45a0fd8d057deb63e7403761b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -1781,7 +1781,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.10.0", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -1792,7 +1792,7 @@ checksum = "29a358ff9f12ec09c3e61fef9b5a9902623a695a46a917b07f269bff1445611a" dependencies = [ "darling_core", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -1854,7 +1854,7 @@ checksum = "67e77553c4162a157adbf834ebae5b415acbecbeafc7a74b0e886657506a7611" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -1978,7 +1978,7 @@ checksum = "a6cbae11b3de8fce2a456e8ea3dada226b35fe791f0dc1d360c0941f0bb681f3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -2084,7 +2084,7 @@ checksum = "03cdc46ec28bd728e67540c528013c6a10eb69a02eb31078a1bda695438cbfb8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -2354,7 +2354,7 @@ checksum = "87750cf4b7a4c0625b1529e4c543c2182106e4dedc60a2a6455e00d212c489ac" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -3642,7 +3642,7 @@ checksum = "ed3955f1a9c7c0c15e092f9c887db08b1fc683305fdf6eb6684f22555355e202" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -3715,7 +3715,7 @@ dependencies = [ "proc-macro-crate 3.1.0", "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -4340,7 +4340,7 @@ checksum = "9e2e25ee72f5b24d773cae88422baddefff7714f97aab68d96fe2b6fc4a28fb2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -5040,7 +5040,7 @@ checksum = "692d6f5ac90220161d6774db30c662202721e64aed9058d2c394f451261420c1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -5095,7 +5095,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -5145,7 +5145,7 @@ checksum = "91d129178576168c589c9ec973feedf7d3126c01ac2bf08795109aa35b69fb8f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -6368,7 +6368,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version 0.4.0", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -6791,7 +6791,7 @@ version = "2.1.0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", "toml 0.8.12", ] @@ -7426,7 +7426,7 @@ dependencies = [ "bs58", "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -8264,7 +8264,7 @@ checksum = "d9e8418ea6269dcfb01c712f0444d2c75542c04448b480e87de59d2865edc750" dependencies = [ "quote", "spl-discriminator-syn", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -8276,7 +8276,7 @@ dependencies = [ "proc-macro2", "quote", "sha2 0.10.8", - "syn 2.0.72", + "syn 2.0.74", "thiserror", ] @@ -8335,7 +8335,7 @@ dependencies = [ "proc-macro2", "quote", "sha2 0.10.8", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -8523,9 +8523,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.72" +version = "2.0.74" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc4b9b9bf2add8093d3f2c0204471e951b2285580335de42f9d2534f3ae7a8af" +checksum = "1fceb41e3d546d0bd83421d3409b1460cc7444cd389341a4c880fe7a042cb3d7" dependencies = [ "proc-macro2", "quote", @@ -8541,7 +8541,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -8727,7 +8727,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -8739,7 +8739,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", "test-case-core", ] @@ -8775,7 +8775,7 @@ checksum = "a4558b58466b9ad7ca0f102865eccc95938dca1a74a856f2b57b6629050da261" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -8912,7 +8912,7 @@ source = "git+https://github.com/anza-xyz/solana-tokio.git?rev=7cf47705faacf7bf0 dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -9156,7 +9156,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -9465,7 +9465,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", "wasm-bindgen-shared", ] @@ -9499,7 +9499,7 @@ checksum = "e94f17b526d0a461a191c78ea52bbce64071ed5c04c9ffe424dcb38f74171bb7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -9849,7 +9849,7 @@ checksum = "b3c129550b3e6de3fd0ba67ba5c81818f9805e58b8d7fee80a3a59d2c9fc601a" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] @@ -9869,7 +9869,7 @@ checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" dependencies = [ "proc-macro2", "quote", - "syn 2.0.72", + "syn 2.0.74", ] [[package]] From e8ddd9ca2933ba56766545646eff190fd3f21088 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Mon, 12 Aug 2024 09:56:30 -0500 Subject: [PATCH 5/5] TransactionView: AddressTableLookupMeta (#2479) --- .../src/address_table_lookup_meta.rs | 201 ++++++++++++++++++ transaction-view/src/lib.rs | 2 + 2 files changed, 203 insertions(+) create mode 100644 transaction-view/src/address_table_lookup_meta.rs diff --git a/transaction-view/src/address_table_lookup_meta.rs b/transaction-view/src/address_table_lookup_meta.rs new file mode 100644 index 00000000000000..4f92547ef2ef7c --- /dev/null +++ b/transaction-view/src/address_table_lookup_meta.rs @@ -0,0 +1,201 @@ +use { + crate::{ + bytes::{ + advance_offset_for_array, advance_offset_for_type, check_remaining, + optimized_read_compressed_u16, read_byte, + }, + result::Result, + }, + solana_sdk::{hash::Hash, packet::PACKET_DATA_SIZE, pubkey::Pubkey, signature::Signature}, +}; + +// Each ATL has at least a Pubkey, one byte for the number of write indexes, +// and one byte for the number of read indexes. Additionally, for validity +// the ATL must have at least one write or read index giving a minimum size +// of 35 bytes. +const MIN_SIZED_ATL: usize = { + core::mem::size_of::() // account key + + 1 // writable indexes length + + 1 // readonly indexes length + + 1 // single account (either write or read) +}; + +// A valid packet with ATLs has: +// 1. At least 1 signature +// 2. 1 message prefix byte +// 3. 3 bytes for the message header +// 4. 1 static account key +// 5. 1 recent blockhash +// 6. 1 byte for the number of instructions (0) +// 7. 1 byte for the number of ATLS +const MIN_SIZED_PACKET_WITH_ATLS: usize = { + 1 // signatures count + + core::mem::size_of::() // signature + + 1 // message prefix + + 3 // message header + + 1 // static account keys count + + core::mem::size_of::() // static account key + + core::mem::size_of::() // recent blockhash + + 1 // number of instructions + + 1 // number of ATLS +}; + +/// The maximum number of ATLS that can fit in a valid packet. +const MAX_ATLS_PER_PACKET: usize = (PACKET_DATA_SIZE - MIN_SIZED_PACKET_WITH_ATLS) / MIN_SIZED_ATL; + +/// Contains metadata about the address table lookups in a transaction packet. +pub struct AddressTableLookupMeta { + /// The number of address table lookups in the transaction. + pub(crate) num_address_table_lookup: u8, + /// The offset to the first address table lookup in the transaction. + pub(crate) offset: u16, +} + +impl AddressTableLookupMeta { + /// Get the number of address table lookups (ATL) and offset to the first. + /// The offset will be updated to point to the first byte after the last + /// ATL. + /// This function will parse each ATL to ensure the data is well-formed, + /// but will not cache data related to these ATLs. + pub fn try_new(bytes: &[u8], offset: &mut usize) -> Result { + // Maximum number of ATLs should be represented by a single byte, + // thus the MSB should not be set. + const _: () = assert!(MAX_ATLS_PER_PACKET & 0b1000_0000 == 0); + let num_address_table_lookups = read_byte(bytes, offset)?; + + // Check that the remaining bytes are enough to hold the ATLs. + check_remaining( + bytes, + *offset, + MIN_SIZED_ATL.wrapping_mul(usize::from(num_address_table_lookups)), + )?; + + // We know the offset does not exceed packet length, and our packet + // length is less than u16::MAX, so we can safely cast to u16. + let address_table_lookups_offset = *offset as u16; + + // The ATLs do not have a fixed size. So we must iterate over + // each ATL to find the total size of the ATLs in the packet, + // and check for any malformed ATLs or buffer overflows. + for _index in 0..num_address_table_lookups { + // Each ATL has 3 pieces: + // 1. Address (Pubkey) + // 2. write indexes ([u8]) + // 3. read indexes ([u8]) + + // Advance offset for address of the lookup table. + advance_offset_for_type::(bytes, offset)?; + + // Read the number of write indexes, and then update the offset. + let num_accounts = optimized_read_compressed_u16(bytes, offset)?; + advance_offset_for_array::(bytes, offset, num_accounts)?; + + // Read the number of read indexes, and then update the offset. + let data_len = optimized_read_compressed_u16(bytes, offset)?; + advance_offset_for_array::(bytes, offset, data_len)? + } + + Ok(Self { + num_address_table_lookup: num_address_table_lookups, + offset: address_table_lookups_offset, + }) + } +} + +#[cfg(test)] +mod tests { + use { + super::*, + solana_sdk::{message::v0::MessageAddressTableLookup, short_vec::ShortVec}, + }; + + #[test] + fn test_zero_atls() { + let bytes = bincode::serialize(&ShortVec::(vec![])).unwrap(); + let mut offset = 0; + let meta = AddressTableLookupMeta::try_new(&bytes, &mut offset).unwrap(); + assert_eq!(meta.num_address_table_lookup, 0); + assert_eq!(meta.offset, 1); + assert_eq!(offset, bytes.len()); + } + + #[test] + fn test_length_too_high() { + let mut bytes = bincode::serialize(&ShortVec::(vec![])).unwrap(); + let mut offset = 0; + // modify the number of atls to be too high + bytes[0] = 5; + assert!(AddressTableLookupMeta::try_new(&bytes, &mut offset).is_err()); + } + + #[test] + fn test_single_atl() { + let bytes = bincode::serialize(&ShortVec::(vec![ + MessageAddressTableLookup { + account_key: Pubkey::new_unique(), + writable_indexes: vec![1, 2, 3], + readonly_indexes: vec![4, 5, 6], + }, + ])) + .unwrap(); + let mut offset = 0; + let meta = AddressTableLookupMeta::try_new(&bytes, &mut offset).unwrap(); + assert_eq!(meta.num_address_table_lookup, 1); + assert_eq!(meta.offset, 1); + assert_eq!(offset, bytes.len()); + } + + #[test] + fn test_multiple_atls() { + let bytes = bincode::serialize(&ShortVec::(vec![ + MessageAddressTableLookup { + account_key: Pubkey::new_unique(), + writable_indexes: vec![1, 2, 3], + readonly_indexes: vec![4, 5, 6], + }, + MessageAddressTableLookup { + account_key: Pubkey::new_unique(), + writable_indexes: vec![1, 2, 3], + readonly_indexes: vec![4, 5, 6], + }, + ])) + .unwrap(); + let mut offset = 0; + let meta = AddressTableLookupMeta::try_new(&bytes, &mut offset).unwrap(); + assert_eq!(meta.num_address_table_lookup, 2); + assert_eq!(meta.offset, 1); + assert_eq!(offset, bytes.len()); + } + + #[test] + fn test_invalid_writable_indexes_vec() { + let mut bytes = bincode::serialize(&ShortVec(vec![MessageAddressTableLookup { + account_key: Pubkey::new_unique(), + writable_indexes: vec![1, 2, 3], + readonly_indexes: vec![4, 5, 6], + }])) + .unwrap(); + + // modify the number of accounts to be too high + bytes[33] = 127; + + let mut offset = 0; + assert!(AddressTableLookupMeta::try_new(&bytes, &mut offset).is_err()); + } + + #[test] + fn test_invalid_readonly_indexes_vec() { + let mut bytes = bincode::serialize(&ShortVec(vec![MessageAddressTableLookup { + account_key: Pubkey::new_unique(), + writable_indexes: vec![1, 2, 3], + readonly_indexes: vec![4, 5, 6], + }])) + .unwrap(); + + // modify the number of accounts to be too high + bytes[37] = 127; + + let mut offset = 0; + assert!(AddressTableLookupMeta::try_new(&bytes, &mut offset).is_err()); + } +} diff --git a/transaction-view/src/lib.rs b/transaction-view/src/lib.rs index 145b4df16ed06c..a9ed7eb9b8f17c 100644 --- a/transaction-view/src/lib.rs +++ b/transaction-view/src/lib.rs @@ -6,6 +6,8 @@ pub mod bytes; #[allow(dead_code)] mod bytes; +#[allow(dead_code)] +mod address_table_lookup_meta; #[allow(dead_code)] mod instructions_meta; #[allow(dead_code)]