Skip to content
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.

Sign up for GitHub

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

Do not check accounts data size in InvokeContext #26773

Merged
merged 2 commits into from
Aug 8, 2022
Merged

Do not check accounts data size in InvokeContext #26773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e34fc47
Do not check accounts data size in InvokeContext
brooksprumo Jul 14, 2022
efda1e1
pr: remove `maximum` field from AccountsDataMeter
brooksprumo Aug 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 2 additions & 129 deletions program-runtime/src/accounts_data_meter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! The accounts data space has a maximum size it is permitted to grow to. This module contains
//! the constants and types for tracking and metering the accounts data space during program
//! runtime.
use solana_sdk::instruction::InstructionError;

/// The maximum allowed size, in bytes, of the accounts data
/// 128 GB was chosen because it is the RAM amount listed under Hardware Recommendations on
Expand All @@ -12,9 +11,6 @@ pub const MAX_ACCOUNTS_DATA_LEN: u64 = 128_000_000_000;
/// Meter and track the amount of available accounts data space
#[derive(Debug, Default, Clone, Copy, Eq, PartialEq)]
pub struct AccountsDataMeter {
/// The maximum amount of accounts data space that can be used (in bytes)
maximum: u64,

/// The initial amount of accounts data space used (in bytes)
initial: u64,

Expand All @@ -26,18 +22,10 @@ impl AccountsDataMeter {
/// Make a new AccountsDataMeter
#[must_use]
pub fn new(initial_accounts_data_len: u64) -> Self {
let accounts_data_meter = Self {
maximum: MAX_ACCOUNTS_DATA_LEN,
Self {
initial: initial_accounts_data_len,
delta: 0,
};
debug_assert!(accounts_data_meter.initial <= accounts_data_meter.maximum);
jstarry marked this conversation as resolved.
Show resolved Hide resolved
accounts_data_meter
}

/// Return the maximum amount of accounts data space that can be used (in bytes)
pub fn maximum(&self) -> u64 {
self.maximum
}
}

/// Return the initial amount of accounts data space used (in bytes)
Expand Down Expand Up @@ -68,41 +56,12 @@ impl AccountsDataMeter {
saturating_add_signed(self.initial, self.delta)
}

/// Get the remaining amount of accounts data space (in bytes)
pub fn remaining(&self) -> u64 {
self.maximum.saturating_sub(self.current())
}

/// Adjust the space used by accounts data by `amount` (in bytes).
///
/// If `amount` is *positive*, we *increase* the space used by accounts data. If `amount` is
/// *negative*, we *decrease* the space used by accounts data. If `amount` is greater than
/// the remaining space, return an error and *do not* adjust accounts data space.
pub fn adjust_delta(&mut self, amount: i64) -> Result<(), InstructionError> {
if amount > self.remaining() as i64 {
return Err(InstructionError::MaxAccountsDataSizeExceeded);
}
self.adjust_delta_unchecked(amount);
Ok(())
}

/// Unconditionally adjust accounts data space. Refer to `adjust_delta()` for more
/// documentation.
pub fn adjust_delta_unchecked(&mut self, amount: i64) {
self.delta = self.delta.saturating_add(amount);
}
}

#[cfg(test)]
impl AccountsDataMeter {
pub fn set_maximum(&mut self, maximum: u64) {
self.maximum = maximum;
}
pub fn set_initial(&mut self, initial: u64) {
self.initial = initial;
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -111,97 +70,11 @@ mod tests {
fn test_new() {
let initial = 1234;
let accounts_data_meter = AccountsDataMeter::new(initial);
assert_eq!(accounts_data_meter.maximum, MAX_ACCOUNTS_DATA_LEN);
assert_eq!(accounts_data_meter.initial, initial);
}

#[test]
fn test_new_can_use_max_len() {
let _ = AccountsDataMeter::new(MAX_ACCOUNTS_DATA_LEN);
}

#[test]
#[should_panic]
fn test_new_panics_if_initial_len_too_big() {
let _ = AccountsDataMeter::new(MAX_ACCOUNTS_DATA_LEN + 1);
}

#[test]
fn test_remaining() {
let initial_accounts_data_len = 0;
let accounts_data_meter = AccountsDataMeter::new(initial_accounts_data_len);
assert_eq!(accounts_data_meter.remaining(), MAX_ACCOUNTS_DATA_LEN);
}

#[test]
fn test_remaining_saturates() {
let initial_accounts_data_len = 0;
let mut accounts_data_meter = AccountsDataMeter::new(initial_accounts_data_len);
// To test that remaining() saturates, need to break the invariant that initial <= maximum
accounts_data_meter.initial = MAX_ACCOUNTS_DATA_LEN + 1;
assert_eq!(accounts_data_meter.remaining(), 0);
}

#[test]
fn test_adjust_delta() {
let initial_accounts_data_len = 0;
let mut accounts_data_meter = AccountsDataMeter::new(initial_accounts_data_len);

// Test: simple, positive numbers
let result = accounts_data_meter.adjust_delta(0);
assert!(result.is_ok());
let result = accounts_data_meter.adjust_delta(1);
assert!(result.is_ok());
let result = accounts_data_meter.adjust_delta(4);
assert!(result.is_ok());
let result = accounts_data_meter.adjust_delta(9);
assert!(result.is_ok());

// Test: adjust_delta can use up the remaining amount
let remaining = accounts_data_meter.remaining() as i64;
let result = accounts_data_meter.adjust_delta(remaining);
assert!(result.is_ok());
assert_eq!(accounts_data_meter.remaining(), 0);
}

#[test]
fn test_adjust_delta_deallocate() {
let initial_accounts_data_len = 10_000;
let mut accounts_data_meter = AccountsDataMeter::new(initial_accounts_data_len);
let remaining_before = accounts_data_meter.remaining();

let amount = (initial_accounts_data_len / 2) as i64;
let amount = -amount;
let result = accounts_data_meter.adjust_delta(amount);
assert!(result.is_ok());
let remaining_after = accounts_data_meter.remaining();
assert_eq!(remaining_after, remaining_before + amount.unsigned_abs());
}

#[test]
fn test_adjust_delta_exceeding() {
let initial_accounts_data_len = 0;
let mut accounts_data_meter = AccountsDataMeter::new(initial_accounts_data_len);

// Test: adjusting delta by more than what's available
// (1) returns an error,
// (2) does not adjust delta
let remaining = accounts_data_meter.remaining();
let result = accounts_data_meter.adjust_delta(remaining as i64 + 1);
assert!(result.is_err());
assert_eq!(accounts_data_meter.remaining(), remaining);
}

#[test]
fn test_adjust_delta_zero() {
// Pre-condition: set up the accounts data meter such that there is no remaining space
let initial_accounts_data_len = 1234;
let mut accounts_data_meter = AccountsDataMeter::new(initial_accounts_data_len);
accounts_data_meter.maximum = initial_accounts_data_len;
assert_eq!(accounts_data_meter.remaining(), 0);

// Test: can always adjust delta by zero, even if there is no remaining space
let result = accounts_data_meter.adjust_delta(0);
assert!(result.is_ok());
}
}
55 changes: 18 additions & 37 deletions program-runtime/src/invoke_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ use {
solana_sdk::{
account::{AccountSharedData, ReadableAccount},
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
feature_set::{
cap_accounts_data_len, enable_early_verification_of_account_modifications, FeatureSet,
},
feature_set::{enable_early_verification_of_account_modifications, FeatureSet},
hash::Hash,
instruction::{AccountMeta, Instruction, InstructionError},
native_loader,
Expand Down Expand Up @@ -448,7 +446,6 @@ impl<'a> InvokeContext<'a> {
instruction_accounts: &[InstructionAccount],
program_indices: &[usize],
) -> Result<(), InstructionError> {
let cap_accounts_data_len = self.feature_set.is_active(&cap_accounts_data_len::id());
let instruction_context = self
.transaction_context
.get_current_instruction_context()
Expand Down Expand Up @@ -519,12 +516,8 @@ impl<'a> InvokeContext<'a> {
let pre_data_len = pre_account.data().len() as i64;
let post_data_len = account.data().len() as i64;
let data_len_delta = post_data_len.saturating_sub(pre_data_len);
if cap_accounts_data_len {
self.accounts_data_meter.adjust_delta(data_len_delta)?;
} else {
self.accounts_data_meter
.adjust_delta_unchecked(data_len_delta);
}
self.accounts_data_meter
.adjust_delta_unchecked(data_len_delta);
}

// Verify that the total sum of all the lamports did not change
Expand All @@ -543,7 +536,6 @@ impl<'a> InvokeContext<'a> {
instruction_accounts: &[InstructionAccount],
before_instruction_context_push: bool,
) -> Result<(), InstructionError> {
let cap_accounts_data_len = self.feature_set.is_active(&cap_accounts_data_len::id());
let transaction_context = &self.transaction_context;
let instruction_context = transaction_context.get_current_instruction_context()?;
let program_id = instruction_context
Expand Down Expand Up @@ -612,12 +604,8 @@ impl<'a> InvokeContext<'a> {
let pre_data_len = pre_account.data().len() as i64;
let post_data_len = account.data().len() as i64;
let data_len_delta = post_data_len.saturating_sub(pre_data_len);
if cap_accounts_data_len {
self.accounts_data_meter.adjust_delta(data_len_delta)?;
} else {
self.accounts_data_meter
.adjust_delta_unchecked(data_len_delta);
}
self.accounts_data_meter
.adjust_delta_unchecked(data_len_delta);

break;
}
Expand Down Expand Up @@ -1519,9 +1507,7 @@ mod tests {
}

#[test]
fn test_process_instruction_accounts_data_meter() {
solana_logger::setup();

fn test_process_instruction_accounts_resize_delta() {
let program_key = Pubkey::new_unique();
let user_account_data_len = 123u64;
let user_account =
Expand All @@ -1545,14 +1531,6 @@ mod tests {
let mut invoke_context =
InvokeContext::new_mock(&mut transaction_context, &builtin_programs);

invoke_context
.accounts_data_meter
.set_initial(user_account_data_len as u64);
invoke_context
.accounts_data_meter
.set_maximum(user_account_data_len as u64 * 3);
let remaining_account_data_len = invoke_context.accounts_data_meter.remaining();

let instruction_accounts = [
InstructionAccount {
index_in_transaction: 0,
Expand All @@ -1570,9 +1548,10 @@ mod tests {
},
];

// Test 1: Resize the account to use up all the space; this must succeed
// Test: Resize the account to *the same size*, so not consuming any additional size; this must succeed
{
let new_len = user_account_data_len + remaining_account_data_len;
let resize_delta: i64 = 0;
let new_len = (user_account_data_len as i64 + resize_delta) as u64;
let instruction_data =
bincode::serialize(&MockInstruction::Resize { new_len }).unwrap();

Expand All @@ -1590,13 +1569,14 @@ mod tests {
.transaction_context
.accounts_resize_delta()
.unwrap(),
user_account_data_len as i64 * 2
resize_delta
);
}

// Test 2: Resize the account to *the same size*, so not consuming any additional size; this must succeed
// Test: Resize the account larger; this must succeed
{
let new_len = user_account_data_len + remaining_account_data_len;
let resize_delta: i64 = 1;
let new_len = (user_account_data_len as i64 + resize_delta) as u64;
let instruction_data =
bincode::serialize(&MockInstruction::Resize { new_len }).unwrap();

Expand All @@ -1614,13 +1594,14 @@ mod tests {
.transaction_context
.accounts_resize_delta()
.unwrap(),
user_account_data_len as i64 * 2
resize_delta
);
}

// Test 3: Resize the account to exceed the budget; this must succeed
// Test: Resize the account smaller; this must succeed
{
let new_len = user_account_data_len + remaining_account_data_len + 1;
let resize_delta: i64 = -1;
let new_len = (user_account_data_len as i64 + resize_delta) as u64;
let instruction_data =
bincode::serialize(&MockInstruction::Resize { new_len }).unwrap();

Expand All @@ -1638,7 +1619,7 @@ mod tests {
.transaction_context
.accounts_resize_delta()
.unwrap(),
user_account_data_len as i64 * 2 + 1
resize_delta
);
}
}
Expand Down