-
Notifications
You must be signed in to change notification settings - Fork 1.1k
SIMD-0339: Increase CPI Account Infos Limit #8513
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
Changes from 24 commits
23f8d97
1c9619b
2f3446d
12b7a09
8a65c68
0d31630
620ff0e
917abad
b0407e6
01466de
df4d61d
8e3e3d1
3c81a81
4629daa
abc7e34
c51ec8c
9071ea7
004ac03
c4b8563
9076cff
4db843a
ac84ebd
3f722bd
bcf5f2b
634f923
f44b4c8
c9e055d
171f59b
3e71a15
4f474f5
7fa3a0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -168,6 +168,7 @@ impl FeatureSet { | |
| raise_cpi_nesting_limit_to_8: self.is_active(&raise_cpi_nesting_limit_to_8::id()), | ||
| provide_instruction_data_offset_in_vm_r2: self | ||
| .is_active(&provide_instruction_data_offset_in_vm_r2::id()), | ||
| increase_cpi_account_info_limit: self.is_active(&increase_cpi_account_info_limit::id()), | ||
| vote_state_v4: self.is_active(&vote_state_v4::id()), | ||
| } | ||
| } | ||
|
|
@@ -1154,7 +1155,6 @@ pub mod static_instruction_limit { | |
| pub mod discard_unexpected_data_complete_shreds { | ||
| solana_pubkey::declare_id!("8MhfKhoZEoiySpVe248bDkisyEcBA7JQLyUS94xoTSqN"); | ||
| } | ||
|
|
||
| pub mod vote_state_v4 { | ||
| solana_pubkey::declare_id!("Gx4XFcrVMt4HUvPzTpTSVkdDVgcDSjKhDN1RqRS6KDuZ"); | ||
|
|
||
|
|
@@ -1167,6 +1167,10 @@ pub mod switch_to_chacha8_turbine { | |
| solana_pubkey::declare_id!("CHaChatUnR3s6cPyPMMGNJa3VdQQ8PNH2JqdD4LpCKnB"); | ||
| } | ||
|
|
||
| pub mod increase_cpi_account_info_limit { | ||
| solana_pubkey::declare_id!("7wM2pdjwmSXviEdsorpcBY3T4YWUPQXDMepZudub7nGQ"); // Placeholder ID HAVE TO CHANGE BEFORE USE | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you update this with a real key? |
||
| } | ||
|
|
||
| pub mod deprecate_rent_exemption_threshold { | ||
| solana_pubkey::declare_id!("rent6iVy6PDoViPBeJ6k5EJQrkj62h7DPyLbWGHwjrC"); | ||
| } | ||
|
|
@@ -2102,6 +2106,10 @@ pub static FEATURE_NAMES: LazyLock<AHashMap<Pubkey, &'static str>> = LazyLock::n | |
| switch_to_chacha8_turbine::id(), | ||
| "SIMD-0332: Reduce ChaCha rounds for Turbine from 20 to 8", | ||
| ), | ||
| ( | ||
| increase_cpi_account_info_limit::id(), | ||
| "SIMD-0339: increase CPI info account limit", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This title doesn't match the SIMD |
||
| ), | ||
| ( | ||
| deprecate_rent_exemption_threshold::id(), | ||
| "SIMD-0194: Deprecate rent exemption threshold", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,12 @@ type Error = Box<dyn std::error::Error>; | |
| const SUCCESS: u64 = 0; | ||
| /// Maximum signers | ||
| const MAX_SIGNERS: usize = 16; | ||
| ///SIMD-0339 based calculation of AccountInfo translation byte size. Fixed size of **80 bytes** for each AccountInfo broken down as: | ||
| /// - 32 bytes for account address | ||
| /// - 32 bytes for owner address | ||
| /// - 8 bytes for lamport balance | ||
| /// - 8 bytes for data length | ||
| const ACCOUNT_INFO_BYTE_SIZE: usize = 80; | ||
|
|
||
| /// Rust representation of C's SolInstruction | ||
| #[derive(Debug)] | ||
|
|
@@ -114,6 +120,8 @@ struct SolSignerSeedsC { | |
|
|
||
| /// Maximum number of account info structs that can be used in a single CPI invocation | ||
| const MAX_CPI_ACCOUNT_INFOS: usize = 128; | ||
| /// Maximum number of account info structs that can be used in a single CPI invocation with SIMD-0339 active | ||
| const MAX_CPI_ACCOUNT_INFOS_SIMD_0339: usize = 255; | ||
|
|
||
| /// Check that an account info pointer field points to the expected address | ||
| fn check_account_info_pointer( | ||
|
|
@@ -158,6 +166,11 @@ fn check_account_infos( | |
| invoke_context: &mut InvokeContext, | ||
| ) -> Result<(), Error> { | ||
| let max_cpi_account_infos = if invoke_context | ||
| .get_feature_set() | ||
| .increase_cpi_account_info_limit | ||
| { | ||
| MAX_CPI_ACCOUNT_INFOS_SIMD_0339 | ||
| } else if invoke_context | ||
| .get_feature_set() | ||
| .increase_tx_account_lock_limit | ||
| { | ||
|
|
@@ -531,14 +544,24 @@ pub fn translate_instruction_rust( | |
| ix.data.as_vaddr(), | ||
| ix.data.len(), | ||
| check_aligned, | ||
| )? | ||
| .to_vec(); | ||
| )?; | ||
|
|
||
| check_instruction_size(account_metas.len(), data.len())?; | ||
|
|
||
| let mut total_cu_translation_cost = data.len() as u64; | ||
|
|
||
| if invoke_context | ||
| .get_feature_set() | ||
| .increase_cpi_account_info_limit | ||
| { | ||
| // Each account meta is 34 bytes (32 for pubkey, 1 for is_signer, 1 for is_writable) | ||
| total_cu_translation_cost = total_cu_translation_cost | ||
| .saturating_add(account_metas.len().saturating_mul(size_of::<AccountMeta>()) as u64); | ||
| } | ||
|
|
||
| consume_compute_meter( | ||
| invoke_context, | ||
| (data.len() as u64) | ||
| (total_cu_translation_cost) | ||
| .checked_div(invoke_context.get_execution_cost().cpi_bytes_per_unit) | ||
| .unwrap_or(u64::MAX), | ||
| )?; | ||
|
|
@@ -559,7 +582,7 @@ pub fn translate_instruction_rust( | |
|
|
||
| Ok(Instruction { | ||
| accounts, | ||
| data, | ||
| data: data.to_vec(), | ||
| program_id: ix.program_id, | ||
| }) | ||
| } | ||
|
|
@@ -650,14 +673,28 @@ pub fn translate_instruction_c( | |
| ix_c.accounts_len, | ||
| check_aligned, | ||
| )?; | ||
| let data = translate_slice::<u8>(memory_mapping, ix_c.data_addr, ix_c.data_len, check_aligned)? | ||
| .to_vec(); | ||
| let data = translate_slice::<u8>(memory_mapping, ix_c.data_addr, ix_c.data_len, check_aligned)?; | ||
|
|
||
| check_instruction_size(ix_c.accounts_len as usize, data.len())?; | ||
|
|
||
| let mut total_cu_translation_cost = data.len() as u64; | ||
|
|
||
| if invoke_context | ||
| .get_feature_set() | ||
| .increase_cpi_account_info_limit | ||
| { | ||
| // Each account meta is 34 bytes (32 for pubkey, 1 for is_signer, 1 for is_writable) | ||
| total_cu_translation_cost = | ||
| total_cu_translation_cost.saturating_add(ix_c.accounts_len.saturating_mul(size_of::< | ||
| AccountMeta, | ||
| >( | ||
| ) | ||
| as u64)); | ||
| } | ||
|
|
||
| consume_compute_meter( | ||
| invoke_context, | ||
| (data.len() as u64) | ||
| (total_cu_translation_cost) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Summing the total data size (ix data + account meta's) before dividing by "cpi bytes per unit" could reduce the impact of rounding error.. and that behavior doesn't match the SIMD. Combining the total cost is fine, just make sure you do the division first before summing the |
||
| .checked_div(invoke_context.get_execution_cost().cpi_bytes_per_unit) | ||
| .unwrap_or(u64::MAX), | ||
| )?; | ||
|
|
@@ -684,7 +721,7 @@ pub fn translate_instruction_c( | |
|
|
||
| Ok(Instruction { | ||
| accounts, | ||
| data, | ||
| data: data.to_vec(), | ||
| program_id: *program_id, | ||
| }) | ||
| } | ||
|
|
@@ -926,6 +963,21 @@ where | |
| check_aligned, | ||
| )?; | ||
| check_account_infos(account_infos.len(), invoke_context)?; | ||
|
|
||
| if invoke_context | ||
| .get_feature_set() | ||
| .increase_cpi_account_info_limit | ||
| { | ||
| let account_infos_bytes = account_infos.len().saturating_mul(ACCOUNT_INFO_BYTE_SIZE); | ||
|
|
||
| consume_compute_meter( | ||
| invoke_context, | ||
| (account_infos_bytes as u64) | ||
| .checked_div(invoke_context.get_execution_cost().cpi_bytes_per_unit) | ||
| .unwrap_or(u64::MAX), | ||
| )?; | ||
| } | ||
|
|
||
| let mut account_info_keys = Vec::with_capacity(account_infos_len as usize); | ||
| #[allow(clippy::needless_range_loop)] | ||
| for account_index in 0..account_infos_len as usize { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is very nit picky but this doesn't look intentional, can you undo this?