Skip to content

Commit

Permalink
Merge pull request #1972 from radixdlt/feature/decrease-buffer-limit
Browse files Browse the repository at this point in the history
Decrease max number of buffers
  • Loading branch information
iamyulong authored Oct 25, 2024
2 parents 662f9be + 659229d commit 469d58f
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 6 deletions.
3 changes: 0 additions & 3 deletions radix-common/src/constants/transaction_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,3 @@ pub const USD_PRICE_IN_XRD: &str = "16.666666666666666666";

/// The maximum that a package or component owner is allowed to set their method royalty to. 10 USD
pub const MAX_PER_FUNCTION_ROYALTY_IN_XRD: &str = "166.666666666666666666";

/// The maximum number of "live" buffers maintained by Scrypto runtime.
pub const MAX_NUMBER_OF_BUFFERS: usize = 32;
7 changes: 7 additions & 0 deletions radix-engine-tests/assets/blueprints/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions radix-engine-tests/assets/blueprints/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ members = [
"oracles/oracle_v3",
"steal",
"locker-factory",
"wasm_limits",
]
resolver = "2"

Expand Down
10 changes: 10 additions & 0 deletions radix-engine-tests/assets/blueprints/wasm_limits/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "wasm_limits"
version = "1.0.0"
edition = "2021"

[dependencies]
scrypto = { path = "../../../../scrypto" }

[lib]
crate-type = ["cdylib", "lib"]
14 changes: 14 additions & 0 deletions radix-engine-tests/assets/blueprints/wasm_limits/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use scrypto::prelude::*;

#[blueprint]
mod wasm_limits {
struct WasmLimits {}

impl WasmLimits {
pub fn create_buffers(n: usize) {
for _ in 0..n {
let _ = unsafe { wasm_api::actor::actor_get_blueprint_name() };
}
}
}
}
1 change: 1 addition & 0 deletions radix-engine-tests/tests/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod scrypto_validation;
mod scrypto_validator;
mod stack_size;
mod system_wasm_buffers;
mod wasm_limits;
mod wasm_memory;
mod wasm_metering;
mod wasm_non_mvp;
Expand Down
43 changes: 43 additions & 0 deletions radix-engine-tests/tests/vm/wasm_limits.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use radix_engine_tests::common::PackageLoader;
use scrypto_test::prelude::*;

#[test]
fn test_create_buffers_within_limits() {
// Arrange
let mut ledger = LedgerSimulatorBuilder::new().build();
let package = ledger.publish_package_simple(PackageLoader::get("wasm_limits"));

let manifest = ManifestBuilder::new()
.lock_fee_from_faucet()
.call_function(package, "WasmLimits", "create_buffers", (4usize,))
.build();

// Act
let receipt = ledger.execute_manifest(manifest, vec![]);

// Assert
receipt.expect_commit_success();
}

#[test]
fn test_crate_buffers_beyond_limits() {
// Arrange
let mut ledger = LedgerSimulatorBuilder::new().build();
let package = ledger.publish_package_simple(PackageLoader::get("wasm_limits"));

let manifest = ManifestBuilder::new()
.lock_fee_from_faucet()
.call_function(package, "WasmLimits", "create_buffers", (5usize,))
.build();

// Act
let receipt = ledger.execute_manifest(manifest, vec![]);

// Assert
receipt.expect_specific_failure(|runtime_error| {
matches!(
runtime_error,
RuntimeError::VmError(VmError::Wasm(WasmRuntimeError::TooManyBuffers))
)
})
}
11 changes: 8 additions & 3 deletions radix-engine/src/vm/wasm_runtime/scrypto_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ pub struct ScryptoRuntime<'y, Y: SystemApi<RuntimeError>> {
package_address: PackageAddress,
export_name: String,
wasm_execution_units_buffer: u32,
max_number_of_buffers: usize,
scrypto_vm_version: ScryptoVmVersion,
}

Expand All @@ -38,7 +37,6 @@ impl<'y, Y: SystemApi<RuntimeError>> ScryptoRuntime<'y, Y> {
package_address,
export_name,
wasm_execution_units_buffer: 0,
max_number_of_buffers: MAX_NUMBER_OF_BUFFERS,
scrypto_vm_version,
}
}
Expand Down Expand Up @@ -83,7 +81,14 @@ impl<'y, Y: SystemApi<RuntimeError>> WasmRuntime for ScryptoRuntime<'y, Y> {
) -> Result<Buffer, InvokeError<WasmRuntimeError>> {
assert!(buffer.len() <= 0xffffffff);

if self.buffers.len() >= self.max_number_of_buffers {
let max_number_of_buffers = match self.scrypto_vm_version {
ScryptoVmVersion::V1_0 | ScryptoVmVersion::V1_1 => 32,
// Practically speaking, there is little gain of keeping multiple buffers open before
// [multi-value](https://github.com/WebAssembly/multi-value/blob/master/proposals/multi-value/Overview.md) is supported and used.
// We reduce it to `4` so that the amount of memory that a transaction can consume is reduced, which is beneficial for parallel execution.
ScryptoVmVersion::V1_2 => 4,
};
if self.buffers.len() >= max_number_of_buffers {
return Err(InvokeError::SelfError(WasmRuntimeError::TooManyBuffers));
}

Expand Down

0 comments on commit 469d58f

Please sign in to comment.