Skip to content

Commit

Permalink
Limit effects size and revert (MystenLabs#9026)
Browse files Browse the repository at this point in the history
## Description 

Limit effects size and rollback when too large.
Rollback still involves applying the gas smashed effects.
We have a soft and hard limit for system TXes. Soft limit alerts and
hard limit terminates the TX.

## Test Plan 
Tested effects generation and temp store.

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [x] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
Serialized effects are limited to 512k in size.
  • Loading branch information
oxade authored Mar 13, 2023
1 parent 47ba7be commit e279ae1
Show file tree
Hide file tree
Showing 9 changed files with 183 additions and 11 deletions.
42 changes: 37 additions & 5 deletions crates/sui-adapter/src/execution_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ use move_core_types::language_storage::ModuleId;
use move_vm_runtime::move_vm::MoveVM;
use sui_types::base_types::ObjectID;
use sui_types::programmable_transaction_builder::ProgrammableTransactionBuilder;
use tracing::{debug, info, instrument};
use tracing::{debug, info, instrument, warn};

use crate::programmable_transactions;
use sui_protocol_config::ProtocolConfig;
use sui_protocol_config::{check_limit_by_meter, LimitThresholdCrossed, ProtocolConfig};
use sui_types::epoch_data::EpochData;
use sui_types::error::ExecutionError;
use sui_types::error::{ExecutionError, ExecutionErrorKind};
use sui_types::gas::GasCostSummary;
use sui_types::messages::{
ConsensusCommitPrologue, GenesisTransaction, ObjectArg, TransactionKind,
Expand Down Expand Up @@ -151,7 +151,7 @@ fn execute_transaction<
// we must still ensure an effect is committed and all objects versions incremented.
let result = charge_gas_for_object_read(temporary_store, &mut gas_status);
let mut result = result.and_then(|()| {
let execution_result = execution_loop::<Mode, _>(
let mut execution_result = execution_loop::<Mode, _>(
temporary_store,
transaction_kind,
gas_object_ref.0,
Expand All @@ -160,8 +160,40 @@ fn execute_transaction<
&mut gas_status,
protocol_config,
);

let effects_estimated_size = temporary_store.estimate_effects_size_upperbound();

// Check if a limit threshold was crossed.
// For metered transactions, there is not soft limit.
// For system transactions, we allow a soft limit with alerting, and a hard limit where we terminate
match check_limit_by_meter!(
!gas_status.is_unmetered(),
effects_estimated_size,
protocol_config.max_serialized_tx_effects_size_bytes(),
protocol_config.max_serialized_tx_effects_size_bytes_system_tx()
) {
LimitThresholdCrossed::None => (),
LimitThresholdCrossed::Soft(_, limit) => {
/* TODO: add more alerting */
warn!(
effects_estimated_size = effects_estimated_size,
soft_limit = limit,
"Estimated transaction effects size crossed soft limit",
)
}
LimitThresholdCrossed::Hard(_, lim) => {
execution_result = Err(ExecutionError::new_with_source(
ExecutionErrorKind::EffectsTooLarge {
current_size: effects_estimated_size as u64,
max_size: lim as u64,
},
"Transaction effects are too large",
))
}
};

if execution_result.is_err() {
// Roll back the temporary store if execution failed.
// Roll back the temporary store if execution failed or effects too large.
temporary_store.reset();
// re-smash so temporary store is again aware of smashing
gas_object_ref = match temporary_store.smash_gas(sender, gas) {
Expand Down
11 changes: 11 additions & 0 deletions crates/sui-core/src/authority/authority_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,19 @@ impl AuthorityStore {
pub fn check_input_objects(
&self,
objects: &[InputObjectKind],
protocol_config: &ProtocolConfig,
) -> Result<Vec<Object>, SuiError> {
let mut result = Vec::new();

fp_ensure!(
objects.len() <= protocol_config.max_input_objects() as usize,
UserInputError::SizeLimitExceeded {
limit: "maximum input objects in a transaction".to_string(),
value: protocol_config.max_input_objects().to_string()
}
.into()
);

for kind in objects {
let obj = match kind {
InputObjectKind::MovePackage(id) | InputObjectKind::SharedMoveObject { id, .. } => {
Expand Down
8 changes: 4 additions & 4 deletions crates/sui-core/src/transaction_input_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub async fn check_transaction_input(
transaction.check_version_supported(epoch_store.protocol_config())?;
transaction.validity_check(epoch_store.protocol_config())?;
let input_objects = transaction.input_objects()?;
let objects = store.check_input_objects(&input_objects)?;
let objects = store.check_input_objects(&input_objects, epoch_store.protocol_config())?;
let gas_status = get_gas_status(&objects, transaction.gas(), epoch_store, transaction).await?;
let input_objects = check_objects(transaction, input_objects, objects)?;
Ok((gas_status, input_objects))
Expand All @@ -71,7 +71,7 @@ pub async fn check_transaction_input_with_given_gas(
transaction.validity_check_no_gas_check(epoch_store.protocol_config())?;

let mut input_objects = transaction.input_objects()?;
let mut objects = store.check_input_objects(&input_objects)?;
let mut objects = store.check_input_objects(&input_objects, epoch_store.protocol_config())?;

let gas_object_ref = gas_object.compute_object_reference();
input_objects.push(InputObjectKind::ImmOrOwnedMoveObject(gas_object_ref));
Expand Down Expand Up @@ -101,7 +101,7 @@ pub(crate) async fn check_dev_inspect_input(
}
}
let mut input_objects = kind.input_objects()?;
let mut objects = store.check_input_objects(&input_objects)?;
let mut objects = store.check_input_objects(&input_objects, config)?;
let mut used_objects: HashSet<SuiAddress> = HashSet::new();
for object in &objects {
if !object.is_immutable() {
Expand Down Expand Up @@ -143,7 +143,7 @@ pub async fn check_certificate_input(
let input_object_data = if tx_data.is_change_epoch_tx() {
// When changing the epoch, we update a the system object, which is shared, without going
// through sequencing, so we must bypass the sequence checks here.
store.check_input_objects(&input_object_kinds)?
store.check_input_objects(&input_object_kinds, epoch_store.protocol_config())?
} else {
store.check_sequenced_input_objects(cert.digest(), &input_object_kinds, epoch_store)?
};
Expand Down
5 changes: 5 additions & 0 deletions crates/sui-core/tests/staged/sui.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@ ExecutionFailureStatus:
- idx: U16
23:
InvalidTransferObject: UNIT
24:
EffectsTooLarge:
STRUCT:
- current_size: U64
- max_size: U64
ExecutionStatus:
ENUM:
0:
Expand Down
26 changes: 25 additions & 1 deletion crates/sui-protocol-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,15 @@ pub struct ProtocolConfig {
/// Maximum number of individual transactions in a Batch transaction.
max_tx_in_batch: Option<u32>,

/// Maximum number of input objects.
max_input_objects: Option<u64>,

/// Maximum size of serialized transaction effects.
max_serialized_tx_effects_size_bytes: Option<u64>,

/// Maximum size of serialized transaction effects for system transactions.
max_serialized_tx_effects_size_bytes_system_tx: Option<u64>,

/// Maximum number of gas payment objets for a transaction.
max_gas_payment_objects: Option<u32>,

Expand Down Expand Up @@ -405,6 +414,17 @@ impl ProtocolConfig {
pub fn max_tx_in_batch(&self) -> u32 {
self.max_tx_in_batch.expect(CONSTANT_ERR_MSG)
}
pub fn max_input_objects(&self) -> u64 {
self.max_input_objects.expect(CONSTANT_ERR_MSG)
}
pub fn max_serialized_tx_effects_size_bytes(&self) -> u64 {
self.max_serialized_tx_effects_size_bytes
.expect(CONSTANT_ERR_MSG)
}
pub fn max_serialized_tx_effects_size_bytes_system_tx(&self) -> u64 {
self.max_serialized_tx_effects_size_bytes_system_tx
.expect(CONSTANT_ERR_MSG)
}
pub fn max_gas_payment_objects(&self) -> u32 {
self.max_gas_payment_objects.expect(CONSTANT_ERR_MSG)
}
Expand Down Expand Up @@ -728,7 +748,11 @@ impl ProtocolConfig {

max_tx_size: Some(64 * 1024),
max_tx_in_batch: Some(10),
max_gas_payment_objects: Some(32),
// We need this number to be at least 100x less than `max_serialized_tx_effects_size_bytes`otherwise effects can be huge
max_input_objects: Some(2048),
max_serialized_tx_effects_size_bytes: Some(512 * 1024),
max_serialized_tx_effects_size_bytes_system_tx: Some(512 * 1024 * 16),
max_gas_payment_objects: Some(256),
max_modules_in_publish: Some(128),
max_arguments: Some(512),
max_type_arguments: Some(16),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ feature_flags:
package_upgrades: false
max_tx_size: 65536
max_tx_in_batch: 10
max_gas_payment_objects: 32
max_input_objects: 2048
max_serialized_tx_effects_size_bytes: 524288
max_serialized_tx_effects_size_bytes_system_tx: 8388608
max_gas_payment_objects: 256
max_modules_in_publish: 128
max_arguments: 512
max_type_arguments: 16
Expand Down
51 changes: 51 additions & 0 deletions crates/sui-types/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,23 @@ pub const DUMMY_GAS_PRICE: u64 = 1;

const BLOCKED_MOVE_FUNCTIONS: [(ObjectID, &str, &str); 0] = [];

// Since `std::mem::size_of` may not be stable across platforms, we use rough constants
// We need these for estimating effects sizes
// Approximate size of `ObjectRef` type in bytes
pub const APPROX_SIZE_OF_OBJECT_REF: usize = 80;
// Approximate size of `ExecutionStatus` type in bytes
pub const APPROX_SIZE_OF_EXECUTION_STATUS: usize = 120;
// Approximate size of `EpochId` type in bytes
pub const APPROX_SIZE_OF_EPOCH_ID: usize = 10;
// Approximate size of `GasCostSummary` type in bytes
pub const APPROX_SIZE_OF_GAS_COST_SUMARY: usize = 30;
// Approximate size of `Option<TransactionEventsDigest>` type in bytes
pub const APPROX_SIZE_OF_OPT_TX_EVENTS_DIGEST: usize = 40;
// Approximate size of `TransactionDigest` type in bytes
pub const APPROX_SIZE_OF_TX_DIGEST: usize = 40;
// Approximate size of `Owner` type in bytes
pub const APPROX_SIZE_OF_OWNER: usize = 48;

#[cfg(test)]
#[path = "unit_tests/messages_tests.rs"]
mod messages_tests;
Expand Down Expand Up @@ -2377,6 +2394,16 @@ pub enum ExecutionFailureStatus {
InvalidPublicFunctionReturnType { idx: u16 },
#[error("Invalid Transfer Object, object does not have public transfer.")]
InvalidTransferObject,

//
// Post-execution errors
//
// Indicates the effects from the transaction are too large
#[error(
"Effects of size {current_size} bytes too large. \
Limit is {max_size} bytes"
)]
EffectsTooLarge { current_size: u64, max_size: u64 },
// NOTE: if you want to add a new enum,
// please add it at the end for Rust SDK backward compatibility.
}
Expand Down Expand Up @@ -2656,6 +2683,30 @@ impl TransactionEffects {
effects: self.digest(),
}
}

pub fn estimate_effects_size_upperbound(
num_writes: usize,
num_mutables: usize,
num_deletes: usize,
num_deps: usize,
) -> usize {
let fixed_sizes = APPROX_SIZE_OF_EXECUTION_STATUS
+ APPROX_SIZE_OF_EPOCH_ID
+ APPROX_SIZE_OF_GAS_COST_SUMARY
+ APPROX_SIZE_OF_OPT_TX_EVENTS_DIGEST;

// Each write or delete contributes at roughly this amount because:
// Each write can be a mutation which can show up in `mutated` and `modified_at_versions`
// `num_delete` is added for padding
let approx_change_entry_size = 1_000
+ (APPROX_SIZE_OF_OWNER + APPROX_SIZE_OF_OBJECT_REF) * num_writes
+ (APPROX_SIZE_OF_OBJECT_REF * num_mutables)
+ (APPROX_SIZE_OF_OBJECT_REF * num_deletes);

let deps_size = 1_000 + APPROX_SIZE_OF_TX_DIGEST * num_deps;

fixed_sizes + approx_change_entry_size + deps_size
}
}

// testing helpers.
Expand Down
10 changes: 10 additions & 0 deletions crates/sui-types/src/temporary_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,16 @@ impl<S> TemporaryStore<S> {
}
}
}

pub fn estimate_effects_size_upperbound(&self) -> usize {
// In the worst case, the number of deps is equal to the number of input objects
TransactionEffects::estimate_effects_size_upperbound(
self.written.len(),
self.mutable_input_refs.len(),
self.deleted.len(),
self.input_objects.len(),
)
}
}

impl<S: GetModule + ObjectStore + BackingPackageStore> TemporaryStore<S> {
Expand Down
36 changes: 36 additions & 0 deletions crates/sui-types/src/unit_tests/messages_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1301,3 +1301,39 @@ fn test_certificate_digest() {
*cert.auth_sig_mut_for_testing() = other_cert.auth_sig().clone();
assert_ne!(digest, cert.certificate_digest());
}

// Use this to ensure that our approximation for components used in effects size are not smaller than expected
// If this test fails, the value of the constant must be increased
#[test]
fn check_approx_effects_components_size() {
use std::mem::size_of;

assert!(
size_of::<GasCostSummary>() < APPROX_SIZE_OF_GAS_COST_SUMARY,
"Update APPROX_SIZE_OF_GAS_COST_SUMARY constant"
);
assert!(
size_of::<EpochId>() < APPROX_SIZE_OF_EPOCH_ID,
"Update APPROX_SIZE_OF_EPOCH_ID constant"
);
assert!(
size_of::<Option<TransactionEventsDigest>>() < APPROX_SIZE_OF_OPT_TX_EVENTS_DIGEST,
"Update APPROX_SIZE_OF_OPT_TX_EVENTS_DIGEST constant"
);
assert!(
size_of::<ObjectRef>() < APPROX_SIZE_OF_OBJECT_REF,
"Update APPROX_SIZE_OF_OBJECT_REF constant"
);
assert!(
size_of::<TransactionDigest>() < APPROX_SIZE_OF_TX_DIGEST,
"Update APPROX_SIZE_OF_TX_DIGEST constant"
);
assert!(
size_of::<Owner>() < APPROX_SIZE_OF_OWNER,
"Update APPROX_SIZE_OF_OWNER constant"
);
assert!(
size_of::<ExecutionStatus>() < APPROX_SIZE_OF_EXECUTION_STATUS,
"Update APPROX_SIZE_OF_EXECUTION_STATUS constant"
);
}

0 comments on commit e279ae1

Please sign in to comment.