From b0c27cf48b5a63842b4ef9e209c0845a53a1f52b Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 25 Jul 2024 17:32:30 +0200 Subject: [PATCH 01/12] Updates gas costs --- crates/gas/src/lib.rs | 42 +++--- crates/vm/src/wasm/run.rs | 270 +++++++++++++++++++------------------- 2 files changed, 156 insertions(+), 156 deletions(-) diff --git a/crates/gas/src/lib.rs b/crates/gas/src/lib.rs index 5cd92511d7..d81c4f089a 100644 --- a/crates/gas/src/lib.rs +++ b/crates/gas/src/lib.rs @@ -50,10 +50,10 @@ pub enum GasParseError { Overflow, } -const COMPILE_GAS_PER_BYTE: u64 = 1_955; +const COMPILE_GAS_PER_BYTE: u64 = 1_664; const PARALLEL_GAS_DIVIDER: u64 = 10; -const WASM_CODE_VALIDATION_GAS_PER_BYTE: u64 = 67; -const WRAPPER_TX_VALIDATION_GAS: u64 = 3_245_500; +const WASM_CODE_VALIDATION_GAS_PER_BYTE: u64 = 59; +const WRAPPER_TX_VALIDATION_GAS: u64 = 1_526_700; // There's no benchmark to calculate the cost of storage occupation, so we // define it as the cost of storage latency (which is needed for any storage // operation and it's based on actual execution time), plus the same cost @@ -73,48 +73,48 @@ const PHYSICAL_STORAGE_LATENCY_PER_BYTE: u64 = 1_000_000; const NETWORK_TRANSMISSION_GAS_PER_BYTE: u64 = 848; /// The cost of accessing data from memory (both read and write mode), per byte -pub const MEMORY_ACCESS_GAS_PER_BYTE: u64 = 104; +pub const MEMORY_ACCESS_GAS_PER_BYTE: u64 = 39; /// The cost of accessing data from storage, per byte pub const STORAGE_ACCESS_GAS_PER_BYTE: u64 = - 163 + PHYSICAL_STORAGE_LATENCY_PER_BYTE; + 93 + PHYSICAL_STORAGE_LATENCY_PER_BYTE; /// The cost of writing data to storage, per byte pub const STORAGE_WRITE_GAS_PER_BYTE: u64 = - MEMORY_ACCESS_GAS_PER_BYTE + 69_634 + STORAGE_OCCUPATION_GAS_PER_BYTE; + MEMORY_ACCESS_GAS_PER_BYTE + 17_583 + STORAGE_OCCUPATION_GAS_PER_BYTE; /// The cost of removing data from storage, per byte pub const STORAGE_DELETE_GAS_PER_BYTE: u64 = - MEMORY_ACCESS_GAS_PER_BYTE + 69_634 + PHYSICAL_STORAGE_LATENCY_PER_BYTE; + MEMORY_ACCESS_GAS_PER_BYTE + 17_583 + PHYSICAL_STORAGE_LATENCY_PER_BYTE; /// The cost of verifying a single signature of a transaction -pub const VERIFY_TX_SIG_GAS: u64 = 594_290; +pub const VERIFY_TX_SIG_GAS: u64 = 435_190; /// The cost for requesting one more page in wasm (64KiB) #[allow(clippy::cast_possible_truncation)] // const in u32 range pub const WASM_MEMORY_PAGE_GAS: u32 = MEMORY_ACCESS_GAS_PER_BYTE as u32 * 64 * 1_024; /// The cost to validate an Ibc action -pub const IBC_ACTION_VALIDATE_GAS: u64 = 1_472_023; +pub const IBC_ACTION_VALIDATE_GAS: u64 = 290_935; /// The cost to execute an Ibc action -pub const IBC_ACTION_EXECUTE_GAS: u64 = 3_678_745; +pub const IBC_ACTION_EXECUTE_GAS: u64 = 1_685_733; /// The cost of masp sig verification -pub const MASP_VERIFY_SIG_GAS: u64 = 5_443_000; +pub const MASP_VERIFY_SIG_GAS: u64 = 3_817_500; /// The fixed cost of spend note verification -pub const MASP_FIXED_SPEND_GAS: u64 = 87_866_000; +pub const MASP_FIXED_SPEND_GAS: u64 = 59_521_000; /// The variable cost of spend note verification -pub const MASP_VARIABLE_SPEND_GAS: u64 = 14_384_000; +pub const MASP_VARIABLE_SPEND_GAS: u64 = 9_849_000; /// The fixed cost of convert note verification -pub const MASP_FIXED_CONVERT_GAS: u64 = 70_308_000; +pub const MASP_FIXED_CONVERT_GAS: u64 = 46_197_000; /// The variable cost of convert note verification -pub const MASP_VARIABLE_CONVERT_GAS: u64 = 12_664_000; +pub const MASP_VARIABLE_CONVERT_GAS: u64 = 10_245_000; /// The fixed cost of output note verification -pub const MASP_FIXED_OUTPUT_GAS: u64 = 78_203_000; +pub const MASP_FIXED_OUTPUT_GAS: u64 = 53_439_000; /// The variable cost of output note verification -pub const MASP_VARIABLE_OUTPUT_GAS: u64 = 14_586_000; +pub const MASP_VARIABLE_OUTPUT_GAS: u64 = 9_710_000; /// The cost to process a masp spend note in the bundle -pub const MASP_SPEND_CHECK_GAS: u64 = 479_730; +pub const MASP_SPEND_CHECK_GAS: u64 = 405_070; /// The cost to process a masp convert note in the bundle -pub const MASP_CONVERT_CHECK_GAS: u64 = 173_570; +pub const MASP_CONVERT_CHECK_GAS: u64 = 188_590; /// The cost to process a masp output note in the bundle -pub const MASP_OUTPUT_CHECK_GAS: u64 = 310_260; +pub const MASP_OUTPUT_CHECK_GAS: u64 = 204_430; /// The cost to run the final masp check in the bundle -pub const MASP_FINAL_CHECK_GAS: u64 = 44; +pub const MASP_FINAL_CHECK_GAS: u64 = 43; /// Gas divider specific for the masp vp. Only allocates half of the cores to /// the masp vp since we can expect the other half to be busy with other vps pub const MASP_PARALLEL_GAS_DIVIDER: u64 = PARALLEL_GAS_DIVIDER / 2; diff --git a/crates/vm/src/wasm/run.rs b/crates/vm/src/wasm/run.rs index dd58a51548..7f78d7968b 100644 --- a/crates/vm/src/wasm/run.rs +++ b/crates/vm/src/wasm/run.rs @@ -790,7 +790,7 @@ impl wasm_instrument::gas_metering::Rules for GasRules { // NOTE: these costs are taken from the benchmarks crate. None of them // should be zero let gas = match instruction { - Unreachable => 129_358, + Unreachable => 57_330, // Just a flag, aribitrary cost of 1 End => 1, // Just a flag, aribitrary cost of 1 @@ -798,45 +798,45 @@ impl wasm_instrument::gas_metering::Rules for GasRules { Nop => 1, Block(_) => 1, Loop(_) => 1, - If(_) => 4, - Br(_) => 27, - BrIf(_) => 36, - BrTable(_) => 70, - Return => 7, - Call(_) => 43, - CallIndirect(_, _) => 140, + If(_) => 5, + Br(_) => 14, + BrIf(_) => 14, + BrTable(_) => 56, + Return => 4, + Call(_) => 16, + CallIndirect(_, _) => 28, Drop => 1, - Select => 37, - GetLocal(_) => 2, + Select => 11, + GetLocal(_) => 1, SetLocal(_) => 2, TeeLocal(_) => 2, - GetGlobal(_) => 3, - SetGlobal(_) => 4, - I32Load(_, _) => 5, - I64Load(_, _) => 5, - F32Load(_, _) => 6, - F64Load(_, _) => 6, - I32Load8S(_, _) => 5, - I32Load8U(_, _) => 5, - I32Load16S(_, _) => 5, - I32Load16U(_, _) => 5, - I64Load8S(_, _) => 5, - I64Load8U(_, _) => 5, - I64Load16S(_, _) => 5, - I64Load16U(_, _) => 5, - I64Load32S(_, _) => 5, - I64Load32U(_, _) => 5, - I32Store(_, _) => 5, - I64Store(_, _) => 7, - F32Store(_, _) => 5, - F64Store(_, _) => 6, - I32Store8(_, _) => 5, - I32Store16(_, _) => 15, - I64Store8(_, _) => 5, - I64Store16(_, _) => 15, - I64Store32(_, _) => 6, - CurrentMemory(_) => 108, - GrowMemory(_) => 394, + GetGlobal(_) => 4, + SetGlobal(_) => 5, + I32Load(_, _) => 8, + I64Load(_, _) => 8, + F32Load(_, _) => 9, + F64Load(_, _) => 9, + I32Load8S(_, _) => 8, + I32Load8U(_, _) => 8, + I32Load16S(_, _) => 8, + I32Load16U(_, _) => 8, + I64Load8S(_, _) => 8, + I64Load8U(_, _) => 8, + I64Load16S(_, _) => 8, + I64Load16U(_, _) => 8, + I64Load32S(_, _) => 7, + I64Load32U(_, _) => 7, + I32Store(_, _) => 8, + I64Store(_, _) => 9, + F32Store(_, _) => 8, + F64Store(_, _) => 9, + I32Store8(_, _) => 7, + I32Store16(_, _) => 13, + I64Store8(_, _) => 7, + I64Store16(_, _) => 12, + I64Store32(_, _) => 8, + CurrentMemory(_) => 110, + GrowMemory(_) => 194, I32Const(_) => 1, I64Const(_) => 1, F32Const(_) => 1, @@ -852,118 +852,118 @@ impl wasm_instrument::gas_metering::Rules for GasRules { I32LeU => 6, I32GeS => 6, I32GeU => 6, - I64Eqz => 7, - I64Eq => 7, - I64Ne => 7, - I64LtS => 7, - I64LtU => 7, - I64GtS => 7, - I64GtU => 7, - I64LeS => 7, - I64LeU => 7, - I64GeS => 7, - I64GeU => 7, - F32Eq => 8, - F32Ne => 8, - F32Lt => 8, - F32Gt => 8, - F32Le => 8, - F32Ge => 8, - F64Eq => 10, - F64Ne => 10, - F64Lt => 9, - F64Gt => 9, - F64Le => 9, - F64Ge => 9, - I32Clz => 35, - I32Ctz => 34, + I64Eqz => 8, + I64Eq => 8, + I64Ne => 8, + I64LtS => 8, + I64LtU => 8, + I64GtS => 8, + I64GtU => 8, + I64LeS => 8, + I64LeU => 8, + I64GeS => 8, + I64GeU => 8, + F32Eq => 10, + F32Ne => 10, + F32Lt => 10, + F32Gt => 9, + F32Le => 10, + F32Ge => 10, + F64Eq => 11, + F64Ne => 11, + F64Lt => 11, + F64Gt => 12, + F64Le => 11, + F64Ge => 11, + I32Clz => 3, + I32Ctz => 3, I32Popcnt => 3, - I32Add => 3, - I32Sub => 3, - I32Mul => 5, - I32DivS => 17, - I32DivU => 17, - I32RemS => 41, - I32RemU => 17, - I32And => 3, - I32Or => 3, - I32Xor => 3, - I32Shl => 3, - I32ShrS => 3, - I32ShrU => 3, - I32Rotl => 3, - I32Rotr => 3, - I64Clz => 35, - I64Ctz => 34, - I64Popcnt => 3, - I64Add => 5, - I64Sub => 5, - I64Mul => 6, - I64DivS => 28, - I64DivU => 28, - I64RemS => 46, - I64RemU => 28, - I64And => 5, - I64Or => 5, - I64Xor => 5, - I64Shl => 4, - I64ShrS => 4, - I64ShrU => 4, - I64Rotl => 4, - I64Rotr => 4, - F32Abs => 4, - F32Neg => 3, - F32Ceil => 6, - F32Floor => 6, - F32Trunc => 6, - F32Nearest => 6, - F32Sqrt => 9, - F32Add => 6, - F32Sub => 6, - F32Mul => 6, - F32Div => 9, - F32Min => 50, - F32Max => 47, - F32Copysign => 6, - F64Abs => 6, - F64Neg => 4, - F64Ceil => 7, - F64Floor => 7, - F64Trunc => 7, - F64Nearest => 7, - F64Sqrt => 17, - F64Add => 7, - F64Sub => 7, - F64Mul => 7, + I32Add => 4, + I32Sub => 4, + I32Mul => 6, + I32DivS => 18, + I32DivU => 18, + I32RemS => 18, + I32RemU => 18, + I32And => 4, + I32Or => 4, + I32Xor => 4, + I32Shl => 4, + I32ShrS => 4, + I32ShrU => 4, + I32Rotl => 4, + I32Rotr => 4, + I64Clz => 4, + I64Ctz => 4, + I64Popcnt => 4, + I64Add => 7, + I64Sub => 7, + I64Mul => 8, + I64DivS => 30, + I64DivU => 30, + I64RemS => 31, + I64RemU => 30, + I64And => 7, + I64Or => 7, + I64Xor => 7, + I64Shl => 6, + I64ShrS => 6, + I64ShrU => 6, + I64Rotl => 6, + I64Rotr => 6, + F32Abs => 5, + F32Neg => 4, + F32Ceil => 7, + F32Floor => 7, + F32Trunc => 7, + F32Nearest => 7, + F32Sqrt => 10, + F32Add => 7, + F32Sub => 7, + F32Mul => 7, + F32Div => 10, + F32Min => 21, + F32Max => 19, + F32Copysign => 9, + F64Abs => 7, + F64Neg => 5, + F64Ceil => 9, + F64Floor => 9, + F64Trunc => 9, + F64Nearest => 9, + F64Sqrt => 19, + F64Add => 9, + F64Sub => 9, + F64Mul => 9, F64Div => 12, - F64Min => 52, - F64Max => 49, - F64Copysign => 11, + F64Min => 24, + F64Max => 31, + F64Copysign => 13, I32WrapI64 => 2, - I32TruncSF32 => 54, - I32TruncUF32 => 54, - I32TruncSF64 => 57, - I32TruncUF64 => 57, - I64ExtendSI32 => 2, + I32TruncSF32 => 24, + I32TruncUF32 => 25, + I32TruncSF64 => 28, + I32TruncUF64 => 27, + I64ExtendSI32 => 3, I64ExtendUI32 => 2, - I64TruncSF32 => 73, - I64TruncUF32 => 70, - I64TruncSF64 => 89, - I64TruncUF64 => 70, + I64TruncSF32 => 24, + I64TruncUF32 => 39, + I64TruncSF64 => 27, + I64TruncUF64 => 46, F32ConvertSI32 => 12, F32ConvertUI32 => 6, F32ConvertSI64 => 6, - F32ConvertUI64 => 39, + F32ConvertUI64 => 12, F32DemoteF64 => 9, F64ConvertSI32 => 12, F64ConvertUI32 => 12, F64ConvertSI64 => 12, - F64ConvertUI64 => 39, + F64ConvertUI64 => 12, F64PromoteF32 => 9, I32ReinterpretF32 => 2, - I64ReinterpretF64 => 2, + I64ReinterpretF64 => 3, F32ReinterpretI32 => 3, - F64ReinterpretI64 => 3, + F64ReinterpretI64 => 4, SignExt(SignExtInstruction::I32Extend8S) => 1, SignExt(SignExtInstruction::I32Extend16S) => 1, SignExt(SignExtInstruction::I64Extend8S) => 1, From 7414874b23375706690e00402be7b4500e483f8e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 7 Aug 2024 17:27:36 +0200 Subject: [PATCH 02/12] Misc minor updates to benchmarks --- crates/benches/native_vps.rs | 128 ++++++++++++++------------------- crates/benches/wasm_opcodes.rs | 57 ++++++++------- 2 files changed, 86 insertions(+), 99 deletions(-) diff --git a/crates/benches/native_vps.rs b/crates/benches/native_vps.rs index c5111a618c..fbc07b5509 100644 --- a/crates/benches/native_vps.rs +++ b/crates/benches/native_vps.rs @@ -224,15 +224,13 @@ fn governance(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!( - governance - .validate_tx( - &signed_tx.to_ref(), - governance.ctx.keys_changed, - governance.ctx.verifiers, - ) - .is_ok() - ) + assert!(governance + .validate_tx( + &signed_tx.to_ref(), + governance.ctx.keys_changed, + governance.ctx.verifiers, + ) + .is_ok()) }) }); } @@ -445,14 +443,13 @@ fn ibc(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!( - ibc.validate_tx( + assert!(ibc + .validate_tx( &signed_tx.to_ref(), ibc.ctx.keys_changed, ibc.ctx.verifiers, ) - .is_ok() - ) + .is_ok()) }) }); } @@ -510,15 +507,13 @@ fn vp_multitoken(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!( - multitoken - .validate_tx( - &signed_tx.to_ref(), - multitoken.ctx.keys_changed, - multitoken.ctx.verifiers, - ) - .is_ok() - ) + assert!(multitoken + .validate_tx( + &signed_tx.to_ref(), + multitoken.ctx.keys_changed, + multitoken.ctx.verifiers, + ) + .is_ok()) }) }); } @@ -619,14 +614,13 @@ fn masp(c: &mut Criterion) { )); b.iter(|| { - assert!( - masp.validate_tx( + assert!(masp + .validate_tx( &signed_tx.to_ref(), masp.ctx.keys_changed, masp.ctx.verifiers, ) - .is_ok() - ); + .is_ok()); }) }); } @@ -751,7 +745,7 @@ fn masp_check_convert(c: &mut Criterion) { } fn masp_check_output(c: &mut Criterion) { - c.bench_function("masp_vp_check_output", |b| { + c.bench_function("vp_masp_check_output", |b| { b.iter_batched( || { let (_, _verifiers_from_tx, signed_tx) = @@ -977,7 +971,7 @@ fn customize_masp_tx_data( ) } -// benchmark the cost of validating two signatures in a batch. +// Benchmark the cost of validating two signatures in a batch (two leverage multiscalar multiplication speedups). The gas cost per single signature verification should be the result of this bench divided by two. fn masp_batch_signature_verification(c: &mut Criterion) { let (_, _, tx) = setup_storage_for_masp_verification("unshielding"); let transaction = tx @@ -1023,8 +1017,7 @@ fn masp_batch_signature_verification(c: &mut Criterion) { // Benchmark both one and two proofs and take the difference as the variable // cost for every proofs. Charge the full cost for the first note and then -// charge the variable cost multiplied by the number of remaining notes and -// divided by the number of cores +// charge the variable cost multiplied by the number of remaining notes fn masp_batch_spend_proofs_validate(c: &mut Criterion) { let mut group = c.benchmark_group("masp_batch_spend_proofs_validate"); let PVKs { spend_vk, .. } = preload_verifying_keys(); @@ -1069,8 +1062,7 @@ fn masp_batch_spend_proofs_validate(c: &mut Criterion) { // Benchmark both one and two proofs and take the difference as the variable // cost for every proofs. Charge the full cost for the first note and then -// charge the variable cost multiplied by the number of remaining notes and -// divided by the number of cores +// charge the variable cost multiplied by the number of remaining notes fn masp_batch_convert_proofs_validate(c: &mut Criterion) { let mut group = c.benchmark_group("masp_batch_convert_proofs_validate"); let PVKs { convert_vk, .. } = preload_verifying_keys(); @@ -1115,8 +1107,7 @@ fn masp_batch_convert_proofs_validate(c: &mut Criterion) { // Benchmark both one and two proofs and take the difference as the variable // cost for every proofs. Charge the full cost for the first note and then -// charge the variable cost multiplied by the number of remaining notes and -// divided by the number of cores +// charge the variable cost multiplied by the number of remaining notes fn masp_batch_output_proofs_validate(c: &mut Criterion) { let mut group = c.benchmark_group("masp_batch_output_proofs_validate"); let PVKs { output_vk, .. } = preload_verifying_keys(); @@ -1232,14 +1223,13 @@ fn pgf(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!( - pgf.validate_tx( + assert!(pgf + .validate_tx( &signed_tx.to_ref(), pgf.ctx.keys_changed, pgf.ctx.verifiers, ) - .is_ok() - ) + .is_ok()) }) }); } @@ -1306,14 +1296,13 @@ fn eth_bridge_nut(c: &mut Criterion) { c.bench_function("vp_eth_bridge_nut", |b| { b.iter(|| { - assert!( - nut.validate_tx( + assert!(nut + .validate_tx( &signed_tx.to_ref(), nut.ctx.keys_changed, nut.ctx.verifiers, ) - .is_ok() - ) + .is_ok()) }) }); } @@ -1376,15 +1365,13 @@ fn eth_bridge(c: &mut Criterion) { c.bench_function("vp_eth_bridge", |b| { b.iter(|| { - assert!( - eth_bridge - .validate_tx( - &signed_tx.to_ref(), - eth_bridge.ctx.keys_changed, - eth_bridge.ctx.verifiers, - ) - .is_ok() - ) + assert!(eth_bridge + .validate_tx( + &signed_tx.to_ref(), + eth_bridge.ctx.keys_changed, + eth_bridge.ctx.verifiers, + ) + .is_ok()) }) }); } @@ -1472,15 +1459,13 @@ fn eth_bridge_pool(c: &mut Criterion) { c.bench_function("vp_eth_bridge_pool", |b| { b.iter(|| { - assert!( - bridge_pool - .validate_tx( - &signed_tx.to_ref(), - bridge_pool.ctx.keys_changed, - bridge_pool.ctx.verifiers, - ) - .is_ok() - ) + assert!(bridge_pool + .validate_tx( + &signed_tx.to_ref(), + bridge_pool.ctx.keys_changed, + bridge_pool.ctx.verifiers, + ) + .is_ok()) }) }); } @@ -1544,15 +1529,13 @@ fn parameters(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!( - parameters - .validate_tx( - &signed_tx.to_ref(), - parameters.ctx.keys_changed, - parameters.ctx.verifiers, - ) - .is_ok() - ) + assert!(parameters + .validate_tx( + &signed_tx.to_ref(), + parameters.ctx.keys_changed, + parameters.ctx.verifiers, + ) + .is_ok()) }) }); } @@ -1619,14 +1602,13 @@ fn pos(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!( - pos.validate_tx( + assert!(pos + .validate_tx( &signed_tx.to_ref(), pos.ctx.keys_changed, pos.ctx.verifiers, ) - .is_ok() - ) + .is_ok()) }) }); } diff --git a/crates/benches/wasm_opcodes.rs b/crates/benches/wasm_opcodes.rs index ceae69bc51..cf93a36fb4 100644 --- a/crates/benches/wasm_opcodes.rs +++ b/crates/benches/wasm_opcodes.rs @@ -24,6 +24,20 @@ use wasmer::{imports, Instance, Module, Store, Value}; const ITERATIONS: u64 = 10_000; const ENTRY_POINT: &str = "op"; +lazy_static! { + static ref EMPTY_MODULE: String = format!( + r#" + (module + (func $f0 nop) + (func $f1 (result i32) i32.const 1 return) + (table 1 funcref) + (elem (i32.const 0) $f0) + (global $iter (mut i32) (i32.const 0)) + (memory 1) + (func (export "{ENTRY_POINT}") (param $local_var i32)"# + ); +} + lazy_static! { static ref WASM_OPTS: Vec = vec![ // Unreachable unconditionally traps, so no need to divide its cost by ITERATIONS because we only execute it once @@ -386,18 +400,7 @@ struct WatBuilder { impl Display for WatBuilder { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!( - f, - r#" - (module - (func $f0 nop) - (func $f1 (result i32) i32.const 1 return) - (table 1 funcref) - (elem (i32.const 0) $f0) - (global $iter (mut i32) (i32.const 0)) - (memory 1) - (func (export "{ENTRY_POINT}") (param $local_var i32)"# - )?; + writeln!(f, r#"{}"#, *EMPTY_MODULE)?; for _ in 0..ITERATIONS { writeln!(f, r#"{}"#, self.wat)?; @@ -420,20 +423,19 @@ fn get_wasm_store() -> Store { // An empty wasm module to serve as the base reference for all the other // instructions since the bigger part of the cost is the function call itself fn empty_module(c: &mut Criterion) { - let module_wat = format!( - r#" - (module - (func (export "{ENTRY_POINT}") (param $local_var i32)) - ) - "#, - ); + let module_wat = format!(r#"{}))"#, *EMPTY_MODULE); let mut store = get_wasm_store(); let module = Module::new(&store, module_wat).unwrap(); let instance = Instance::new(&mut store, &module, &imports! {}).unwrap(); - let function = instance.exports.get_function(ENTRY_POINT).unwrap(); + let function = instance + .exports + .get_function(ENTRY_POINT) + .unwrap() + .typed::(&store) + .unwrap(); c.bench_function("empty_module", |b| { - b.iter(|| function.call(&mut store, &[Value::I32(0)]).unwrap()); + b.iter(|| function.call(&mut store, 0).unwrap()); }); } @@ -445,15 +447,18 @@ fn ops(c: &mut Criterion) { let module = Module::new(&store, builder.to_string()).unwrap(); let instance = Instance::new(&mut store, &module, &imports! {}).unwrap(); - let function = instance.exports.get_function(ENTRY_POINT).unwrap(); + let function = instance + .exports + .get_function(ENTRY_POINT) + .unwrap() + .typed::(&store) + .unwrap(); group.bench_function(format!("{}", builder.instruction), |b| { if let Unreachable = builder.instruction { - b.iter(|| { - function.call(&mut store, &[Value::I32(0)]).unwrap_err() - }); + b.iter(|| function.call(&mut store, 0).unwrap_err()); } else { - b.iter(|| function.call(&mut store, &[Value::I32(0)]).unwrap()); + b.iter(|| function.call(&mut store, 0).unwrap()); } }); } From c03ff7d9a9fdce67f0e7336fea7d8117c4cb66ca Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 7 Aug 2024 17:28:25 +0200 Subject: [PATCH 03/12] Adjusts gas costs for wasm --- crates/vm/src/wasm/run.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/vm/src/wasm/run.rs b/crates/vm/src/wasm/run.rs index 7f78d7968b..252e24f656 100644 --- a/crates/vm/src/wasm/run.rs +++ b/crates/vm/src/wasm/run.rs @@ -759,10 +759,17 @@ where } Commitment::Id(code) => { let tx_len = code.len() as u64; + // FIXME: can remove this gas? I can remove it only if I find a way + // to discriminate between normal txs and governacne txs. For + // governacne txs I run this piece of code, for the others I don't + // and I don't need to charge the gas gas_meter .borrow_mut() .add_wasm_validation_gas(tx_len) .map_err(|e| Error::GasError(e.to_string()))?; + // Validation is only needed for governance proposals. The other + // transactions are subject to the allowlist and are guaranteed to + // not contain invalid opcodes. validate_untrusted_wasm(code).map_err(Error::ValidationError)?; gas_meter @@ -790,13 +797,19 @@ impl wasm_instrument::gas_metering::Rules for GasRules { // NOTE: these costs are taken from the benchmarks crate. None of them // should be zero let gas = match instruction { - Unreachable => 57_330, - // Just a flag, aribitrary cost of 1 + // NOTE: the real cost of this operation is 57_330 but because of + // the behavior of the instrumentaiton tools which doesn't account + // for traps in called functions we need to reduce it to 1 otherwise + // the gas costs explode + Unreachable => 1, + // Just a label, aribitrary cost of 1 End => 1, - // Just a flag, aribitrary cost of 1 + // Just a label, aribitrary cost of 1 Else => 1, Nop => 1, + // Just a label, cost of 1 Block(_) => 1, + // Just a label, cost of 1 Loop(_) => 1, If(_) => 5, Br(_) => 14, @@ -986,7 +999,7 @@ impl wasm_instrument::gas_metering::Rules for GasRules { } fn call_per_local_cost(&self) -> u32 { - 1 + 0 } } From 55c230dc8efa35d50dd7fd3b65e2af21b6756caa Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 7 Aug 2024 17:29:44 +0200 Subject: [PATCH 04/12] Disables gas dividers and updates wrong costs --- crates/gas/src/lib.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/gas/src/lib.rs b/crates/gas/src/lib.rs index d81c4f089a..6741a4ae0b 100644 --- a/crates/gas/src/lib.rs +++ b/crates/gas/src/lib.rs @@ -51,7 +51,7 @@ pub enum GasParseError { } const COMPILE_GAS_PER_BYTE: u64 = 1_664; -const PARALLEL_GAS_DIVIDER: u64 = 10; +const PARALLEL_GAS_DIVIDER: u64 = 1; const WASM_CODE_VALIDATION_GAS_PER_BYTE: u64 = 59; const WRAPPER_TX_VALIDATION_GAS: u64 = 1_526_700; // There's no benchmark to calculate the cost of storage occupation, so we @@ -68,7 +68,7 @@ const STORAGE_OCCUPATION_GAS_PER_BYTE: u64 = // storage blob but this would make it more tedious to compute gas in the // codebase. For these two reasons we just set an arbitrary value (based on // actual SSDs latency) per byte here -const PHYSICAL_STORAGE_LATENCY_PER_BYTE: u64 = 1_000_000; +const PHYSICAL_STORAGE_LATENCY_PER_BYTE: u64 = 20; // This is based on the global average bandwidth const NETWORK_TRANSMISSION_GAS_PER_BYTE: u64 = 848; @@ -94,7 +94,7 @@ pub const IBC_ACTION_VALIDATE_GAS: u64 = 290_935; /// The cost to execute an Ibc action pub const IBC_ACTION_EXECUTE_GAS: u64 = 1_685_733; /// The cost of masp sig verification -pub const MASP_VERIFY_SIG_GAS: u64 = 3_817_500; +pub const MASP_VERIFY_SIG_GAS: u64 = 1_908_750; /// The fixed cost of spend note verification pub const MASP_FIXED_SPEND_GAS: u64 = 59_521_000; /// The variable cost of spend note verification @@ -117,7 +117,7 @@ pub const MASP_OUTPUT_CHECK_GAS: u64 = 204_430; pub const MASP_FINAL_CHECK_GAS: u64 = 43; /// Gas divider specific for the masp vp. Only allocates half of the cores to /// the masp vp since we can expect the other half to be busy with other vps -pub const MASP_PARALLEL_GAS_DIVIDER: u64 = PARALLEL_GAS_DIVIDER / 2; +pub const MASP_PARALLEL_GAS_DIVIDER: u64 = 1; /// Gas module result for functions that may fail pub type Result = std::result::Result; @@ -470,12 +470,19 @@ impl VpGasMeter { current_gas: Gas::default(), } } + + // FIXME: remove + pub fn get_vp_consumed_gas(&self) -> Gas { + self.current_gas + } } impl VpsGas { /// Set the gas cost from a VP run. It consumes the [`VpGasMeter`] /// instance which shouldn't be accessed passed this point. pub fn set(&mut self, vp_gas_meter: VpGasMeter) -> Result<()> { + // FIXME: remove + eprintln!("GAS USED ONLY BY VP: {:#?}", vp_gas_meter.current_gas); if vp_gas_meter.current_gas > self.max { self.rest.push(self.max); self.max = vp_gas_meter.current_gas; From 63af02af1e801b36d03b97c9c0cda0186cf27f57 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 7 Aug 2024 18:03:48 +0200 Subject: [PATCH 05/12] Adds note to wrapper benchmark --- crates/benches/native_vps.rs | 119 +++++++++++++++++------------- crates/benches/process_wrapper.rs | 9 ++- 2 files changed, 76 insertions(+), 52 deletions(-) diff --git a/crates/benches/native_vps.rs b/crates/benches/native_vps.rs index fbc07b5509..bceadf78a8 100644 --- a/crates/benches/native_vps.rs +++ b/crates/benches/native_vps.rs @@ -224,13 +224,15 @@ fn governance(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!(governance - .validate_tx( - &signed_tx.to_ref(), - governance.ctx.keys_changed, - governance.ctx.verifiers, - ) - .is_ok()) + assert!( + governance + .validate_tx( + &signed_tx.to_ref(), + governance.ctx.keys_changed, + governance.ctx.verifiers, + ) + .is_ok() + ) }) }); } @@ -443,13 +445,14 @@ fn ibc(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!(ibc - .validate_tx( + assert!( + ibc.validate_tx( &signed_tx.to_ref(), ibc.ctx.keys_changed, ibc.ctx.verifiers, ) - .is_ok()) + .is_ok() + ) }) }); } @@ -507,13 +510,15 @@ fn vp_multitoken(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!(multitoken - .validate_tx( - &signed_tx.to_ref(), - multitoken.ctx.keys_changed, - multitoken.ctx.verifiers, - ) - .is_ok()) + assert!( + multitoken + .validate_tx( + &signed_tx.to_ref(), + multitoken.ctx.keys_changed, + multitoken.ctx.verifiers, + ) + .is_ok() + ) }) }); } @@ -614,13 +619,14 @@ fn masp(c: &mut Criterion) { )); b.iter(|| { - assert!(masp - .validate_tx( + assert!( + masp.validate_tx( &signed_tx.to_ref(), masp.ctx.keys_changed, masp.ctx.verifiers, ) - .is_ok()); + .is_ok() + ); }) }); } @@ -971,7 +977,9 @@ fn customize_masp_tx_data( ) } -// Benchmark the cost of validating two signatures in a batch (two leverage multiscalar multiplication speedups). The gas cost per single signature verification should be the result of this bench divided by two. +// Benchmark the cost of validating two signatures in a batch (two leverage +// multiscalar multiplication speedups). The gas cost per single signature +// verification should be the result of this bench divided by two. fn masp_batch_signature_verification(c: &mut Criterion) { let (_, _, tx) = setup_storage_for_masp_verification("unshielding"); let transaction = tx @@ -1223,13 +1231,14 @@ fn pgf(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!(pgf - .validate_tx( + assert!( + pgf.validate_tx( &signed_tx.to_ref(), pgf.ctx.keys_changed, pgf.ctx.verifiers, ) - .is_ok()) + .is_ok() + ) }) }); } @@ -1296,13 +1305,14 @@ fn eth_bridge_nut(c: &mut Criterion) { c.bench_function("vp_eth_bridge_nut", |b| { b.iter(|| { - assert!(nut - .validate_tx( + assert!( + nut.validate_tx( &signed_tx.to_ref(), nut.ctx.keys_changed, nut.ctx.verifiers, ) - .is_ok()) + .is_ok() + ) }) }); } @@ -1365,13 +1375,15 @@ fn eth_bridge(c: &mut Criterion) { c.bench_function("vp_eth_bridge", |b| { b.iter(|| { - assert!(eth_bridge - .validate_tx( - &signed_tx.to_ref(), - eth_bridge.ctx.keys_changed, - eth_bridge.ctx.verifiers, - ) - .is_ok()) + assert!( + eth_bridge + .validate_tx( + &signed_tx.to_ref(), + eth_bridge.ctx.keys_changed, + eth_bridge.ctx.verifiers, + ) + .is_ok() + ) }) }); } @@ -1459,13 +1471,15 @@ fn eth_bridge_pool(c: &mut Criterion) { c.bench_function("vp_eth_bridge_pool", |b| { b.iter(|| { - assert!(bridge_pool - .validate_tx( - &signed_tx.to_ref(), - bridge_pool.ctx.keys_changed, - bridge_pool.ctx.verifiers, - ) - .is_ok()) + assert!( + bridge_pool + .validate_tx( + &signed_tx.to_ref(), + bridge_pool.ctx.keys_changed, + bridge_pool.ctx.verifiers, + ) + .is_ok() + ) }) }); } @@ -1529,13 +1543,15 @@ fn parameters(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!(parameters - .validate_tx( - &signed_tx.to_ref(), - parameters.ctx.keys_changed, - parameters.ctx.verifiers, - ) - .is_ok()) + assert!( + parameters + .validate_tx( + &signed_tx.to_ref(), + parameters.ctx.keys_changed, + parameters.ctx.verifiers, + ) + .is_ok() + ) }) }); } @@ -1602,13 +1618,14 @@ fn pos(c: &mut Criterion) { group.bench_function(bench_name, |b| { b.iter(|| { - assert!(pos - .validate_tx( + assert!( + pos.validate_tx( &signed_tx.to_ref(), pos.ctx.keys_changed, pos.ctx.verifiers, ) - .is_ok()) + .is_ok() + ) }) }); } diff --git a/crates/benches/process_wrapper.rs b/crates/benches/process_wrapper.rs index fd608e500d..3f4047f826 100644 --- a/crates/benches/process_wrapper.rs +++ b/crates/benches/process_wrapper.rs @@ -84,6 +84,13 @@ fn process_tx(c: &mut Criterion) { )| { assert_eq!( // Assert that the wrapper transaction was valid + // NOTE: this function invovles a loop on the inner txs to + // check that they are allowlisted. The cost of that should + // technically depend on the number of inner txs and should + // be computed at runtime. From some tests though, we can + // see that the cost of that operation is minimale (200 + // ns), so we can just approximate it to a constant cost + // included in this benchmark shell .check_proposal_tx( &wrapper, @@ -104,5 +111,5 @@ fn process_tx(c: &mut Criterion) { }); } -criterion_group!(process_wrapper, process_tx); +criterion_group!(process_wrapper, process_tx,); criterion_main!(process_wrapper); From ea6f5ea49ea853bc0d15d93d288a840f20b85caf Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 8 Aug 2024 11:24:29 +0200 Subject: [PATCH 06/12] Gas correction for non-wasm --- crates/gas/src/lib.rs | 151 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 124 insertions(+), 27 deletions(-) diff --git a/crates/gas/src/lib.rs b/crates/gas/src/lib.rs index 6741a4ae0b..5076bc212b 100644 --- a/crates/gas/src/lib.rs +++ b/crates/gas/src/lib.rs @@ -50,74 +50,169 @@ pub enum GasParseError { Overflow, } -const COMPILE_GAS_PER_BYTE: u64 = 1_664; -const PARALLEL_GAS_DIVIDER: u64 = 1; -const WASM_CODE_VALIDATION_GAS_PER_BYTE: u64 = 59; -const WRAPPER_TX_VALIDATION_GAS: u64 = 1_526_700; +// RAW GAS COSTS +// ================================================================================ +// These are the raw gas costs exctracted from the benchmarks. +// + +const COMPILE_GAS_PER_BYTE_RAW: u64 = 1_664; +const WASM_CODE_VALIDATION_GAS_PER_BYTE_RAW: u64 = 59; +const WRAPPER_TX_VALIDATION_GAS_RAW: u64 = 1_526_700; // There's no benchmark to calculate the cost of storage occupation, so we // define it as the cost of storage latency (which is needed for any storage // operation and it's based on actual execution time), plus the same cost // multiplied by an arbitrary factor that represents the higher cost of storage // space as a resource. This way, the storage occupation cost is not completely // free-floating but it's tied to the other costs -const STORAGE_OCCUPATION_GAS_PER_BYTE: u64 = - PHYSICAL_STORAGE_LATENCY_PER_BYTE * (1 + 10); +const STORAGE_OCCUPATION_GAS_PER_BYTE_RAW: u64 = + PHYSICAL_STORAGE_LATENCY_PER_BYTE_RAW * (1 + 10); // NOTE: this accounts for the latency of a physical drive access. For read // accesses we have no way to tell if data was in cache or in storage. Moreover, // the latency shouldn't really be accounted per single byte but rather per // storage blob but this would make it more tedious to compute gas in the // codebase. For these two reasons we just set an arbitrary value (based on // actual SSDs latency) per byte here -const PHYSICAL_STORAGE_LATENCY_PER_BYTE: u64 = 20; +const PHYSICAL_STORAGE_LATENCY_PER_BYTE_RAW: u64 = 20; // This is based on the global average bandwidth -const NETWORK_TRANSMISSION_GAS_PER_BYTE: u64 = 848; +const NETWORK_TRANSMISSION_GAS_PER_BYTE_RAW: u64 = 848; + +// The cost of accessing data from memory (both read and write mode), per byte +const MEMORY_ACCESS_GAS_PER_BYTE_RAW: u64 = 39; +// The cost of accessing data from storage, per byte +const STORAGE_ACCESS_GAS_PER_BYTE_RAW: u64 = + 93 + PHYSICAL_STORAGE_LATENCY_PER_BYTE_RAW; +// The cost of writing data to storage, per byte +const STORAGE_WRITE_GAS_PER_BYTE_RAW: u64 = MEMORY_ACCESS_GAS_PER_BYTE_RAW + + 17_583 + + STORAGE_OCCUPATION_GAS_PER_BYTE_RAW; +// The cost of removing data from storage, per byte +const STORAGE_DELETE_GAS_PER_BYTE_RAW: u64 = MEMORY_ACCESS_GAS_PER_BYTE_RAW + + 17_583 + + PHYSICAL_STORAGE_LATENCY_PER_BYTE_RAW; +// The cost of verifying a single signature of a transaction +const VERIFY_TX_SIG_GAS_RAW: u64 = 435_190; +// The cost for requesting one more page in wasm (64KiB) +const WASM_MEMORY_PAGE_GAS_RAW: u64 = + MEMORY_ACCESS_GAS_PER_BYTE_RAW * 64 * 1_024; +// The cost to validate an Ibc action +const IBC_ACTION_VALIDATE_GAS_RAW: u64 = 290_935; +// The cost to execute an Ibc action +const IBC_ACTION_EXECUTE_GAS_RAW: u64 = 1_685_733; +// The cost of masp sig verification +const MASP_VERIFY_SIG_GAS_RAW: u64 = 1_908_750; +// The fixed cost of spend note verification +const MASP_FIXED_SPEND_GAS_RAW: u64 = 59_521_000; +// The variable cost of spend note verification +const MASP_VARIABLE_SPEND_GAS_RAW: u64 = 9_849_000; +// The fixed cost of convert note verification +const MASP_FIXED_CONVERT_GAS_RAW: u64 = 46_197_000; +// The variable cost of convert note verification +const MASP_VARIABLE_CONVERT_GAS_RAW: u64 = 10_245_000; +// The fixed cost of output note verification +const MASP_FIXED_OUTPUT_GAS_RAW: u64 = 53_439_000; +// The variable cost of output note verification +const MASP_VARIABLE_OUTPUT_GAS_RAW: u64 = 9_710_000; +// The cost to process a masp spend note in the bundle +const MASP_SPEND_CHECK_GAS_RAW: u64 = 405_070; +// The cost to process a masp convert note in the bundle +const MASP_CONVERT_CHECK_GAS_RAW: u64 = 188_590; +// The cost to process a masp output note in the bundle +const MASP_OUTPUT_CHECK_GAS_RAW: u64 = 204_430; +// The cost to run the final masp check in the bundle +const MASP_FINAL_CHECK_GAS_RAW: u64 = 43; +// ================================================================================ + +// This is a correction factor for non-WASM-opcodes costs. We can see that the +// gas cost we get for wasm codes (txs and vps) is much greater than what we +// would expect from the benchmarks. This is likely due to some imperfections in +// the injection tool but, most importantly, to the fact that the code we end up +// executing is an optimized version of the one we instrument. Therefore we +// provide this factor to correct the costs of non-WASM gas based on the avarage +// speedup we can observe. NOTE: we should really reduce the gas costs of WASM +// opcodes instead of increasing the gas costs of non-WASM gas, but the former +// would involve some complicated adjustments for host function calls so we +// prefer to go with the latter. +const GAS_COST_CORRECTION: u64 = 5; + +// FIXME: actually, can we use a macro for this? + +// ADJUSTED GAS COSTS +// ================================================================================ +// These are the gas costs adjusted for the correction factor. +// +const PARALLEL_GAS_DIVIDER: u64 = 1; +const COMPILE_GAS_PER_BYTE: u64 = + COMPILE_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; +const WASM_CODE_VALIDATION_GAS_PER_BYTE: u64 = + WASM_CODE_VALIDATION_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; +const WRAPPER_TX_VALIDATION_GAS: u64 = + WRAPPER_TX_VALIDATION_GAS_RAW * GAS_COST_CORRECTION; +const STORAGE_OCCUPATION_GAS_PER_BYTE: u64 = + STORAGE_OCCUPATION_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; +const NETWORK_TRANSMISSION_GAS_PER_BYTE: u64 = + NETWORK_TRANSMISSION_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; /// The cost of accessing data from memory (both read and write mode), per byte -pub const MEMORY_ACCESS_GAS_PER_BYTE: u64 = 39; +pub const MEMORY_ACCESS_GAS_PER_BYTE: u64 = + MEMORY_ACCESS_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; /// The cost of accessing data from storage, per byte pub const STORAGE_ACCESS_GAS_PER_BYTE: u64 = - 93 + PHYSICAL_STORAGE_LATENCY_PER_BYTE; + STORAGE_ACCESS_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; /// The cost of writing data to storage, per byte pub const STORAGE_WRITE_GAS_PER_BYTE: u64 = - MEMORY_ACCESS_GAS_PER_BYTE + 17_583 + STORAGE_OCCUPATION_GAS_PER_BYTE; + STORAGE_WRITE_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; /// The cost of removing data from storage, per byte pub const STORAGE_DELETE_GAS_PER_BYTE: u64 = - MEMORY_ACCESS_GAS_PER_BYTE + 17_583 + PHYSICAL_STORAGE_LATENCY_PER_BYTE; + STORAGE_DELETE_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; /// The cost of verifying a single signature of a transaction -pub const VERIFY_TX_SIG_GAS: u64 = 435_190; +pub const VERIFY_TX_SIG_GAS: u64 = VERIFY_TX_SIG_GAS_RAW * GAS_COST_CORRECTION; /// The cost for requesting one more page in wasm (64KiB) #[allow(clippy::cast_possible_truncation)] // const in u32 range pub const WASM_MEMORY_PAGE_GAS: u32 = - MEMORY_ACCESS_GAS_PER_BYTE as u32 * 64 * 1_024; + (WASM_MEMORY_PAGE_GAS_RAW * GAS_COST_CORRECTION) as u32; /// The cost to validate an Ibc action -pub const IBC_ACTION_VALIDATE_GAS: u64 = 290_935; +pub const IBC_ACTION_VALIDATE_GAS: u64 = + IBC_ACTION_VALIDATE_GAS_RAW * GAS_COST_CORRECTION; /// The cost to execute an Ibc action -pub const IBC_ACTION_EXECUTE_GAS: u64 = 1_685_733; +pub const IBC_ACTION_EXECUTE_GAS: u64 = + IBC_ACTION_EXECUTE_GAS_RAW * GAS_COST_CORRECTION; /// The cost of masp sig verification -pub const MASP_VERIFY_SIG_GAS: u64 = 1_908_750; +pub const MASP_VERIFY_SIG_GAS: u64 = + MASP_VERIFY_SIG_GAS_RAW * GAS_COST_CORRECTION; /// The fixed cost of spend note verification -pub const MASP_FIXED_SPEND_GAS: u64 = 59_521_000; +pub const MASP_FIXED_SPEND_GAS: u64 = + MASP_FIXED_SPEND_GAS_RAW * GAS_COST_CORRECTION; /// The variable cost of spend note verification -pub const MASP_VARIABLE_SPEND_GAS: u64 = 9_849_000; +pub const MASP_VARIABLE_SPEND_GAS: u64 = + MASP_VARIABLE_SPEND_GAS_RAW * GAS_COST_CORRECTION; /// The fixed cost of convert note verification -pub const MASP_FIXED_CONVERT_GAS: u64 = 46_197_000; +pub const MASP_FIXED_CONVERT_GAS: u64 = + MASP_FIXED_CONVERT_GAS_RAW * GAS_COST_CORRECTION; /// The variable cost of convert note verification -pub const MASP_VARIABLE_CONVERT_GAS: u64 = 10_245_000; +pub const MASP_VARIABLE_CONVERT_GAS: u64 = + MASP_VARIABLE_CONVERT_GAS_RAW * GAS_COST_CORRECTION; /// The fixed cost of output note verification -pub const MASP_FIXED_OUTPUT_GAS: u64 = 53_439_000; +pub const MASP_FIXED_OUTPUT_GAS: u64 = + MASP_FIXED_OUTPUT_GAS_RAW * GAS_COST_CORRECTION; /// The variable cost of output note verification -pub const MASP_VARIABLE_OUTPUT_GAS: u64 = 9_710_000; +pub const MASP_VARIABLE_OUTPUT_GAS: u64 = + MASP_VARIABLE_OUTPUT_GAS_RAW * GAS_COST_CORRECTION; /// The cost to process a masp spend note in the bundle -pub const MASP_SPEND_CHECK_GAS: u64 = 405_070; +pub const MASP_SPEND_CHECK_GAS: u64 = + MASP_SPEND_CHECK_GAS_RAW * GAS_COST_CORRECTION; /// The cost to process a masp convert note in the bundle -pub const MASP_CONVERT_CHECK_GAS: u64 = 188_590; +pub const MASP_CONVERT_CHECK_GAS: u64 = + MASP_CONVERT_CHECK_GAS_RAW * GAS_COST_CORRECTION; /// The cost to process a masp output note in the bundle -pub const MASP_OUTPUT_CHECK_GAS: u64 = 204_430; +pub const MASP_OUTPUT_CHECK_GAS: u64 = + MASP_OUTPUT_CHECK_GAS_RAW * GAS_COST_CORRECTION; /// The cost to run the final masp check in the bundle -pub const MASP_FINAL_CHECK_GAS: u64 = 43; +pub const MASP_FINAL_CHECK_GAS: u64 = + MASP_FINAL_CHECK_GAS_RAW * GAS_COST_CORRECTION; /// Gas divider specific for the masp vp. Only allocates half of the cores to /// the masp vp since we can expect the other half to be busy with other vps pub const MASP_PARALLEL_GAS_DIVIDER: u64 = 1; +// ================================================================================ /// Gas module result for functions that may fail pub type Result = std::result::Result; @@ -252,6 +347,7 @@ pub trait GasMetering { /// Add the compiling cost proportionate to the code length fn add_compiling_gas(&mut self, bytes_len: u64) -> Result<()> { + // FIXME: need discount here self.consume( bytes_len .checked_mul(COMPILE_GAS_PER_BYTE) @@ -261,6 +357,7 @@ pub trait GasMetering { /// Add the gas for loading the wasm code from storage fn add_wasm_load_from_storage_gas(&mut self, bytes_len: u64) -> Result<()> { + // FIXME: need discount here self.consume( bytes_len .checked_mul(STORAGE_ACCESS_GAS_PER_BYTE) From 8327624d44a9ab79ae8ec8f535b5a17b9270e3e4 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 8 Aug 2024 14:00:25 +0200 Subject: [PATCH 07/12] Reduces compilation gas costs --- crates/gas/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/gas/src/lib.rs b/crates/gas/src/lib.rs index 5076bc212b..de0bf7ab5c 100644 --- a/crates/gas/src/lib.rs +++ b/crates/gas/src/lib.rs @@ -142,8 +142,10 @@ const GAS_COST_CORRECTION: u64 = 5; // const PARALLEL_GAS_DIVIDER: u64 = 1; +// The compilation cost is reduced by a factor to compensate for the (most +// likely) presence of the cache const COMPILE_GAS_PER_BYTE: u64 = - COMPILE_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; + COMPILE_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION / 100; const WASM_CODE_VALIDATION_GAS_PER_BYTE: u64 = WASM_CODE_VALIDATION_GAS_PER_BYTE_RAW * GAS_COST_CORRECTION; const WRAPPER_TX_VALIDATION_GAS: u64 = @@ -347,7 +349,6 @@ pub trait GasMetering { /// Add the compiling cost proportionate to the code length fn add_compiling_gas(&mut self, bytes_len: u64) -> Result<()> { - // FIXME: need discount here self.consume( bytes_len .checked_mul(COMPILE_GAS_PER_BYTE) @@ -357,7 +358,6 @@ pub trait GasMetering { /// Add the gas for loading the wasm code from storage fn add_wasm_load_from_storage_gas(&mut self, bytes_len: u64) -> Result<()> { - // FIXME: need discount here self.consume( bytes_len .checked_mul(STORAGE_ACCESS_GAS_PER_BYTE) From fc208bc5c6cfbcb47a1d222470f79b9134db14e8 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 8 Aug 2024 14:28:42 +0200 Subject: [PATCH 08/12] Updates gas scale and default gas limit --- crates/sdk/src/lib.rs | 2 +- genesis/localnet/parameters.toml | 4 ++-- genesis/starter/parameters.toml | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 463a4fd153..f8ee403a88 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -82,7 +82,7 @@ use tx::{ use wallet::{Wallet, WalletIo, WalletStorage}; /// Default gas-limit -pub const DEFAULT_GAS_LIMIT: u64 = 100_000; +pub const DEFAULT_GAS_LIMIT: u64 = 150_000; #[allow(missing_docs)] #[cfg(not(feature = "async-send"))] diff --git a/genesis/localnet/parameters.toml b/genesis/localnet/parameters.toml index ef76cfb4c3..5d58fdfb1a 100644 --- a/genesis/localnet/parameters.toml +++ b/genesis/localnet/parameters.toml @@ -19,11 +19,11 @@ epochs_per_year = 31_536_000 # The multiplier for masp epochs masp_epoch_multiplier = 2 # Max gas for block -max_block_gas = 20000000 +max_block_gas = 15_000_000 # Masp fee payment gas limit masp_fee_payment_gas_limit = 150_000 # Gas scale -gas_scale = 10_000_000 +gas_scale = 10_000 # Map of the cost per gas unit for every token allowed for fee payment [parameters.minimum_gas_price] diff --git a/genesis/starter/parameters.toml b/genesis/starter/parameters.toml index 2b4f5ea052..6be2fc6d7c 100644 --- a/genesis/starter/parameters.toml +++ b/genesis/starter/parameters.toml @@ -19,11 +19,11 @@ epochs_per_year = 31_536_000 # The multiplier for masp epochs masp_epoch_multiplier = 2 # Max gas for block -max_block_gas = 20000000 +max_block_gas = 15_000_000 # Masp fee payment gas limit masp_fee_payment_gas_limit = 150_000 # Gas scale -gas_scale = 10_000_000 +gas_scale = 10_000 # Map of the cost per gas unit for every token allowed for fee payment [parameters.minimum_gas_price] From d5fcaca8de61b1bd3ba8de50c9bc4647089d3e95 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 8 Aug 2024 14:54:15 +0200 Subject: [PATCH 09/12] Clippy + fmt --- crates/benches/wasm_opcodes.rs | 2 +- crates/gas/src/lib.rs | 17 +++-------------- crates/vm/src/wasm/run.rs | 4 ---- 3 files changed, 4 insertions(+), 19 deletions(-) diff --git a/crates/benches/wasm_opcodes.rs b/crates/benches/wasm_opcodes.rs index cf93a36fb4..add5d4dd14 100644 --- a/crates/benches/wasm_opcodes.rs +++ b/crates/benches/wasm_opcodes.rs @@ -17,7 +17,7 @@ use wasm_instrument::parity_wasm::elements::Instruction::*; use wasm_instrument::parity_wasm::elements::{ BlockType, BrTableData, SignExtInstruction, }; -use wasmer::{imports, Instance, Module, Store, Value}; +use wasmer::{imports, Instance, Module, Store}; // Don't reduce this value too much or it will be impossible to see the // differences in execution times between the diffent instructions diff --git a/crates/gas/src/lib.rs b/crates/gas/src/lib.rs index de0bf7ab5c..d3d78e8419 100644 --- a/crates/gas/src/lib.rs +++ b/crates/gas/src/lib.rs @@ -52,9 +52,8 @@ pub enum GasParseError { // RAW GAS COSTS // ================================================================================ -// These are the raw gas costs exctracted from the benchmarks. +// The raw gas costs exctracted from the benchmarks. // - const COMPILE_GAS_PER_BYTE_RAW: u64 = 1_664; const WASM_CODE_VALIDATION_GAS_PER_BYTE_RAW: u64 = 59; const WRAPPER_TX_VALIDATION_GAS_RAW: u64 = 1_526_700; @@ -122,7 +121,7 @@ const MASP_OUTPUT_CHECK_GAS_RAW: u64 = 204_430; const MASP_FINAL_CHECK_GAS_RAW: u64 = 43; // ================================================================================ -// This is a correction factor for non-WASM-opcodes costs. We can see that the +// A correction factor for non-WASM-opcodes costs. We can see that the // gas cost we get for wasm codes (txs and vps) is much greater than what we // would expect from the benchmarks. This is likely due to some imperfections in // the injection tool but, most importantly, to the fact that the code we end up @@ -134,13 +133,10 @@ const MASP_FINAL_CHECK_GAS_RAW: u64 = 43; // prefer to go with the latter. const GAS_COST_CORRECTION: u64 = 5; -// FIXME: actually, can we use a macro for this? - // ADJUSTED GAS COSTS // ================================================================================ -// These are the gas costs adjusted for the correction factor. +// The gas costs adjusted for the correction factor. // - const PARALLEL_GAS_DIVIDER: u64 = 1; // The compilation cost is reduced by a factor to compensate for the (most // likely) presence of the cache @@ -567,19 +563,12 @@ impl VpGasMeter { current_gas: Gas::default(), } } - - // FIXME: remove - pub fn get_vp_consumed_gas(&self) -> Gas { - self.current_gas - } } impl VpsGas { /// Set the gas cost from a VP run. It consumes the [`VpGasMeter`] /// instance which shouldn't be accessed passed this point. pub fn set(&mut self, vp_gas_meter: VpGasMeter) -> Result<()> { - // FIXME: remove - eprintln!("GAS USED ONLY BY VP: {:#?}", vp_gas_meter.current_gas); if vp_gas_meter.current_gas > self.max { self.rest.push(self.max); self.max = vp_gas_meter.current_gas; diff --git a/crates/vm/src/wasm/run.rs b/crates/vm/src/wasm/run.rs index 252e24f656..d22e7b84c8 100644 --- a/crates/vm/src/wasm/run.rs +++ b/crates/vm/src/wasm/run.rs @@ -759,10 +759,6 @@ where } Commitment::Id(code) => { let tx_len = code.len() as u64; - // FIXME: can remove this gas? I can remove it only if I find a way - // to discriminate between normal txs and governacne txs. For - // governacne txs I run this piece of code, for the others I don't - // and I don't need to charge the gas gas_meter .borrow_mut() .add_wasm_validation_gas(tx_len) From b5e269f02700c8777a804fbd45a8e4470b47f859 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 8 Aug 2024 18:42:37 +0200 Subject: [PATCH 10/12] Fixes integration tests --- crates/tests/src/integration/ledger_tests.rs | 2 +- crates/tests/src/integration/masp.rs | 2 ++ genesis/localnet/parameters.toml | 2 +- genesis/starter/parameters.toml | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/tests/src/integration/ledger_tests.rs b/crates/tests/src/integration/ledger_tests.rs index 5b995e16de..f24213ee4b 100644 --- a/crates/tests/src/integration/ledger_tests.rs +++ b/crates/tests/src/integration/ledger_tests.rs @@ -718,7 +718,7 @@ fn proposal_submission() -> Result<()> { "--data-path", valid_proposal_json_path.to_str().unwrap(), "--gas-limit", - "2000000", + "5000000", "--node", &validator_one_rpc, ]); diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index e719c58c1c..b9f6854647 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -685,6 +685,8 @@ fn masp_incentives() -> Result<()> { CHRISTEL, "--token", NAM, + "--gas-limit", + "200000", "--amount", "1.451732", "--signing-keys", diff --git a/genesis/localnet/parameters.toml b/genesis/localnet/parameters.toml index 5d58fdfb1a..66cbefe091 100644 --- a/genesis/localnet/parameters.toml +++ b/genesis/localnet/parameters.toml @@ -21,7 +21,7 @@ masp_epoch_multiplier = 2 # Max gas for block max_block_gas = 15_000_000 # Masp fee payment gas limit -masp_fee_payment_gas_limit = 150_000 +masp_fee_payment_gas_limit = 200_000 # Gas scale gas_scale = 10_000 diff --git a/genesis/starter/parameters.toml b/genesis/starter/parameters.toml index 6be2fc6d7c..143c68fd5d 100644 --- a/genesis/starter/parameters.toml +++ b/genesis/starter/parameters.toml @@ -21,7 +21,7 @@ masp_epoch_multiplier = 2 # Max gas for block max_block_gas = 15_000_000 # Masp fee payment gas limit -masp_fee_payment_gas_limit = 150_000 +masp_fee_payment_gas_limit = 200_000 # Gas scale gas_scale = 10_000 From f487a379ab31373e6dafd80dfd02486902c2ffc1 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 25 Jul 2024 17:38:36 +0200 Subject: [PATCH 11/12] Changelog #3554 --- .changelog/unreleased/improvements/3554-rc-gas-update.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/3554-rc-gas-update.md diff --git a/.changelog/unreleased/improvements/3554-rc-gas-update.md b/.changelog/unreleased/improvements/3554-rc-gas-update.md new file mode 100644 index 0000000000..93ecd0abc2 --- /dev/null +++ b/.changelog/unreleased/improvements/3554-rc-gas-update.md @@ -0,0 +1,2 @@ +- Updated the gas costs based on benchmarks ran on v41. + ([\#3554](https://github.com/anoma/namada/pull/3554)) \ No newline at end of file From a1be239b05f832a6baa037a50a949ab3f666cd06 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 8 Aug 2024 22:32:12 +0200 Subject: [PATCH 12/12] Fixes e2e tests --- crates/tests/src/e2e/ibc_tests.rs | 6 +++--- crates/tests/src/e2e/ledger_tests.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index fb01648738..aa11f8565f 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -1968,7 +1968,7 @@ fn submit_ibc_tx( "--gas-token", NAM, "--gas-limit", - "150000", + "200000", "--node", &rpc ]), @@ -2017,7 +2017,7 @@ fn transfer( "--port-id", &port_id, "--gas-limit", - "150000", + "200000", "--node", &rpc, ]); @@ -2165,7 +2165,7 @@ fn propose_inflation(test: &Test) -> Result { "--data-path", proposal_json_path.to_str().unwrap(), "--gas-limit", - "2000000", + "4000000", "--node", &rpc, ]); diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index 488fb386f3..f2fb82dd99 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -1972,7 +1972,7 @@ fn proposal_change_shielded_reward() -> Result<()> { "--data-path", valid_proposal_json_path.to_str().unwrap(), "--gas-limit", - "2000000", + "4000000", "--node", &validator_one_rpc, ]);