diff --git a/client-sdk/ts-web/rt/playground/src/consensus.js b/client-sdk/ts-web/rt/playground/src/consensus.js index 8ceb514df8..8b3648f8e6 100644 --- a/client-sdk/ts-web/rt/playground/src/consensus.js +++ b/client-sdk/ts-web/rt/playground/src/consensus.js @@ -207,8 +207,7 @@ export const playground = (async function () { }) .setSignerInfo([siAlice1]) .setFeeAmount(FEE_FREE) - .setFeeGas(0n) - .setFeeConsensusMessages(1); + .setFeeGas(78n); // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 128). await twDeposit.sign([csAlice], consensusChainContext); const addrAliceBech32 = oasis.staking.addressToBech32(aliceAddr); @@ -282,8 +281,7 @@ export const playground = (async function () { }) .setSignerInfo([siDave2]) .setFeeAmount(FEE_FREE) - .setFeeGas(0n) - .setFeeConsensusMessages(1); + .setFeeGas(78n); // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 128). await twWithdraw.sign([csDave], consensusChainContext); /** @type {oasisRT.types.ConsensusAccountsWithdrawEvent} */ diff --git a/runtime-sdk/modules/evm/src/raw_tx.rs b/runtime-sdk/modules/evm/src/raw_tx.rs index 5d78a61dda..bb06182ce9 100644 --- a/runtime-sdk/modules/evm/src/raw_tx.rs +++ b/runtime-sdk/modules/evm/src/raw_tx.rs @@ -177,8 +177,7 @@ pub fn decode( fee: transaction::Fee { amount: token::BaseUnits(resolved_fee_amount, token::Denomination::NATIVE), gas: gas_limit, - // TODO: Allow customization, maybe through call data? - consensus_messages: 1, + consensus_messages: 0, // Dynamic number of consensus messages, limited by gas. }, ..Default::default() }, diff --git a/runtime-sdk/src/modules/consensus/mod.rs b/runtime-sdk/src/modules/consensus/mod.rs index ca2c1a6e73..35b934fab9 100644 --- a/runtime-sdk/src/modules/consensus/mod.rs +++ b/runtime-sdk/src/modules/consensus/mod.rs @@ -36,7 +36,7 @@ use crate::{ address::{Address, SignatureAddressSpec}, message::MessageEventHookInvocation, token, - transaction::AddressSpec, + transaction::{AddressSpec, CallerAddress}, }, Runtime, }; @@ -384,6 +384,15 @@ impl API for Module { fn ensure_compatible_tx_signer() -> Result<(), Error> { CurrentState::with_env(|env| match env.tx_auth_info().signer_info[0].address_spec { AddressSpec::Signature(SignatureAddressSpec::Ed25519(_)) => Ok(()), + AddressSpec::Internal(CallerAddress::Address(_)) if env.is_simulation() => { + // During simulations, the caller may be overriden in case of confidential runtimes + // which would cause this check to always fail, making gas estimation incorrect. + // + // Note that this is optimistic as a `CallerAddres::Address(_)` can still be + // incompatible, but as long as this is only allowed during simulations it shouldn't + // result in any problems. + Ok(()) + } _ => Err(Error::ConsensusIncompatibleSigner), }) } diff --git a/runtime-sdk/src/modules/core/mod.rs b/runtime-sdk/src/modules/core/mod.rs index 36f137e9fc..4d2c24ee1b 100644 --- a/runtime-sdk/src/modules/core/mod.rs +++ b/runtime-sdk/src/modules/core/mod.rs @@ -1106,7 +1106,7 @@ impl module::TransactionHandler for Module { } fn after_handle_call( - _ctx: &C, + ctx: &C, result: module::CallResult, ) -> Result { // Skip handling for internally generated calls. @@ -1114,30 +1114,51 @@ impl module::TransactionHandler for Module { return Ok(result); } - // Charge storage update gas cost if this would be greater than the gas use. let params = Self::params(); - if params.gas_costs.storage_byte > 0 { + + // Compute storage update gas cost. + let storage_gas = if params.gas_costs.storage_byte > 0 { let storage_update_bytes = CurrentState::with(|state| state.pending_store_update_byte_size()); - let storage_gas = params + params .gas_costs .storage_byte - .saturating_mul(storage_update_bytes as u64); - let used_gas = Self::used_tx_gas(); + .saturating_mul(storage_update_bytes as u64) + } else { + 0 + }; - if storage_gas > used_gas { - Self::use_tx_gas(storage_gas - used_gas)?; - } - } + // Compute message gas cost. + let message_gas = { + let emitted_message_count = + CurrentState::with(|state| state.emitted_messages_local_count()); + // Determine how much each message emission costs based on max_batch_gas and the number + // of messages that can be emitted per batch. + let message_gas_cost = params + .max_batch_gas + .checked_div(ctx.max_messages().into()) + .unwrap_or(u64::MAX); // If no messages are allowed, cost is infinite. + message_gas_cost.saturating_mul(emitted_message_count as u64) + }; + + // Compute the gas amount that the transaction should pay in the end. + let used_gas = Self::used_tx_gas(); + let max_gas = std::cmp::max(used_gas, std::cmp::max(storage_gas, message_gas)); // Emit gas used event (if this is not an internally generated call). if Cfg::EMIT_GAS_USED_EVENTS { - let used_gas = Self::used_tx_gas(); CurrentState::with(|state| { - state.emit_unconditional_event(Event::GasUsed { amount: used_gas }); + state.emit_unconditional_event(Event::GasUsed { amount: max_gas }); }); } + // Make sure the transaction actually pays for the maximum gas. Note that failure here is + // fine since the extra resources (storage updates or emitted consensus messages) have not + // actually been spent yet (this happens at the end of the round). + if max_gas > used_gas { + Self::use_tx_gas(max_gas - used_gas)?; + } + Ok(result) } } diff --git a/runtime-sdk/src/modules/core/test.rs b/runtime-sdk/src/modules/core/test.rs index c1ac0e5db8..8cdd8399fb 100644 --- a/runtime-sdk/src/modules/core/test.rs +++ b/runtime-sdk/src/modules/core/test.rs @@ -4,7 +4,10 @@ use once_cell::unsync::Lazy; use crate::{ context::Context, - core::common::version::Version, + core::{ + common::{version::Version, versioned::Versioned}, + consensus::{roothash, staking}, + }, crypto::multisig, error::Error, event::IntoTags, @@ -16,7 +19,10 @@ use crate::{ sender::SenderMeta, state::{self, CurrentState, Options}, testing::{configmap, keys, mock}, - types::{address::Address, token, transaction, transaction::CallerAddress}, + types::{ + address::Address, message::MessageEventHookInvocation, token, transaction, + transaction::CallerAddress, + }, }; use super::{types, Event, Parameters, API as _}; @@ -206,6 +212,7 @@ impl GasWasterModule { const METHOD_SPECIFIC_GAS_REQUIRED_HUGE: &'static str = "test.SpecificGasRequiredHuge"; const METHOD_STORAGE_UPDATE: &'static str = "test.StorageUpdate"; const METHOD_STORAGE_REMOVE: &'static str = "test.StorageRemove"; + const METHOD_EMIT_CONSENSUS_MESSAGE: &'static str = "test.EmitConsensusMessage"; } #[sdk_derive(Module)] @@ -318,6 +325,27 @@ impl GasWasterModule { CurrentState::with_store(|store| store.remove(&args)); Ok(()) } + + #[handler(call = Self::METHOD_EMIT_CONSENSUS_MESSAGE)] + fn emit_consensus_message( + ctx: &C, + count: u64, + ) -> Result<(), ::Error> { + ::Core::use_tx_gas(2)?; + CurrentState::with(|state| { + for _ in 0..count { + state.emit_message( + ctx, + roothash::Message::Staking(Versioned::new( + 0, + roothash::StakingMessage::Transfer(staking::Transfer::default()), + )), + MessageEventHookInvocation::new("test".to_string(), ""), + )?; + } + Ok(()) + }) + } } impl module::BlockHandler for GasWasterModule {} @@ -1125,6 +1153,7 @@ fn test_module_info() { MethodHandlerInfo { kind: types::MethodHandlerKind::Call, name: "test.SpecificGasRequiredHuge".to_string() }, MethodHandlerInfo { kind: types::MethodHandlerKind::Call, name: "test.StorageUpdate".to_string() }, MethodHandlerInfo { kind: types::MethodHandlerKind::Call, name: "test.StorageRemove".to_string() }, + MethodHandlerInfo { kind: types::MethodHandlerKind::Call, name: "test.EmitConsensusMessage".to_string() }, ], }, } @@ -1376,3 +1405,127 @@ fn test_storage_gas() { assert_eq!(events.len(), 1); // Just one gas used event. assert_eq!(events[0].amount, expected_gas_use); } + +#[test] +fn test_message_gas() { + let mut mock = mock::Mock::default(); + let max_messages = 32; + mock.max_messages = max_messages; + + let ctx = mock.create_ctx_for_runtime::(false); + + GasWasterRuntime::migrate(&ctx); + + let max_batch_gas = 10_000; + Core::set_params(Parameters { + max_batch_gas, + gas_costs: super::GasCosts { + tx_byte: 0, + ..Default::default() + }, + ..Core::params() + }); + + let mut signer = mock::Signer::new(0, keys::alice::sigspec()); + + // Emit 10 messages which is greater than the transaction compute gas cost. + let num_messages = 10u64; + let mut total_messages = num_messages; + let expected_gas_use = num_messages * (max_batch_gas / (max_messages as u64)); + let dispatch_result = signer.call_opts( + &ctx, + GasWasterModule::METHOD_EMIT_CONSENSUS_MESSAGE, + num_messages, + mock::CallOptions { + fee: transaction::Fee { + gas: 10_000, + ..Default::default() + }, + }, + ); + assert!(dispatch_result.result.is_success(), "call should succeed"); + + // Simulate multiple transactions in a batch by not taking any messages. + + let tags = &dispatch_result.tags; + assert_eq!(tags.len(), 1, "one event should have been emitted"); + assert_eq!(tags[0].key, b"core\x00\x00\x00\x01"); // core.GasUsed (code = 1) event + + #[derive(Debug, Default, cbor::Decode)] + struct GasUsedEvent { + amount: u64, + } + + let events: Vec = cbor::from_slice(&tags[0].value).unwrap(); + assert_eq!(events.len(), 1); // Just one gas used event. + assert_eq!(events[0].amount, expected_gas_use); + + // Emit no messages so just the compute gas cost should be charged. + let num_messages = 0u64; + total_messages += num_messages; + let expected_gas_use = 2; // Just compute gas cost. + let dispatch_result = signer.call_opts( + &ctx, + GasWasterModule::METHOD_EMIT_CONSENSUS_MESSAGE, + num_messages, + mock::CallOptions { + fee: transaction::Fee { + gas: 10_000, + ..Default::default() + }, + }, + ); + assert!(dispatch_result.result.is_success(), "call should succeed"); + + let tags = &dispatch_result.tags; + assert_eq!(tags.len(), 1, "one event should have been emitted"); + assert_eq!(tags[0].key, b"core\x00\x00\x00\x01"); // core.GasUsed (code = 1) event + + let events: Vec = cbor::from_slice(&tags[0].value).unwrap(); + assert_eq!(events.len(), 1); // Just one gas used event. + assert_eq!(events[0].amount, expected_gas_use); + + // Take all messages emitted by the above two transactions. + let messages = CurrentState::with(|state| state.take_messages()); + assert_eq!(total_messages as usize, messages.len()); + + // Ensure gas estimation works. + let num_messages = 10; + let expected_gas_use = num_messages * (max_batch_gas / (max_messages as u64)); + let tx = transaction::Transaction { + version: 1, + call: transaction::Call { + format: transaction::CallFormat::Plain, + method: GasWasterModule::METHOD_EMIT_CONSENSUS_MESSAGE.to_owned(), + body: cbor::to_value(num_messages), + ..Default::default() + }, + auth_info: transaction::AuthInfo { + signer_info: vec![transaction::SignerInfo::new_sigspec( + keys::alice::sigspec(), + 0, + )], + fee: transaction::Fee { + amount: token::BaseUnits::new(0, token::Denomination::NATIVE), + gas: u64::MAX, + consensus_messages: 0, + }, + ..Default::default() + }, + }; + let estimated_gas: u64 = signer + .query( + &ctx, + "core.EstimateGas", + types::EstimateGasQuery { + caller: None, + tx, + propagate_failures: false, + }, + ) + .expect("gas estimation should succeed"); + assert_eq!( + estimated_gas, expected_gas_use, + "gas should be estimated correctly" + ); +} diff --git a/runtime-sdk/src/state.rs b/runtime-sdk/src/state.rs index 71a3c00cd1..ce477ee8ac 100644 --- a/runtime-sdk/src/state.rs +++ b/runtime-sdk/src/state.rs @@ -483,7 +483,8 @@ impl State { self.hidden_block_values = Some(mem::take(&mut self.block_values)); } - /// Emitted messages count returns the number of messages emitted so far. + /// Emitted messages count returns the number of messages emitted so far across this and all + /// parent states. pub fn emitted_messages_count(&self) -> usize { self.messages.len() + self @@ -493,10 +494,21 @@ impl State { .unwrap_or_default() } + /// Emitted messages count returns the number of messages emitted so far in this state, not + /// counting any parent states. + pub fn emitted_messages_local_count(&self) -> usize { + self.messages.len() + } + /// Maximum number of messages that can be emitted. pub fn emitted_messages_max(&self, ctx: &C) -> u32 { if self.env.is_transaction() { - self.env.tx_auth_info().fee.consensus_messages + let limit = self.env.tx_auth_info().fee.consensus_messages; + if limit > 0 { + limit + } else { + ctx.max_messages() // Zero means an implicit limit by gas use. + } } else { ctx.max_messages() } @@ -1168,6 +1180,9 @@ mod test { CurrentState::with(|state| { state.open(); + assert_eq!(state.emitted_messages_count(), 0); + assert_eq!(state.emitted_messages_local_count(), 0); + state .emit_message( &ctx, @@ -1179,10 +1194,13 @@ mod test { ) .expect("message emission should succeed"); assert_eq!(state.emitted_messages_count(), 1); + assert_eq!(state.emitted_messages_local_count(), 1); assert_eq!(state.emitted_messages_max(&ctx), max_messages as u32); state.open(); // Start child state. + assert_eq!(state.emitted_messages_local_count(), 0); + state .emit_message( &ctx, @@ -1194,6 +1212,7 @@ mod test { ) .expect("message emission should succeed"); assert_eq!(state.emitted_messages_count(), 2); + assert_eq!(state.emitted_messages_local_count(), 1); assert_eq!(state.emitted_messages_max(&ctx), max_messages as u32); state.rollback(); // Rollback. @@ -1203,9 +1222,12 @@ mod test { 1, "emitted message should have been rolled back" ); + assert_eq!(state.emitted_messages_local_count(), 1); state.open(); // Start child state. + assert_eq!(state.emitted_messages_local_count(), 0); + state .emit_message( &ctx, @@ -1217,6 +1239,7 @@ mod test { ) .expect("message emission should succeed"); assert_eq!(state.emitted_messages_count(), 2); + assert_eq!(state.emitted_messages_local_count(), 1); state.commit(); // Commit. @@ -1267,6 +1290,14 @@ mod test { assert_eq!(state.emitted_messages_max(&ctx), 1); }); }); + + let mut tx = mock::transaction(); + tx.auth_info.fee.consensus_messages = 0; // Zero means an implicit limit by gas use. + CurrentState::with_transaction_opts(Options::new().with_tx(tx.into()), || { + CurrentState::with(|state| { + assert_eq!(state.emitted_messages_max(&ctx), max_messages as u32); + }); + }); } #[test] diff --git a/runtime-sdk/src/types/transaction.rs b/runtime-sdk/src/types/transaction.rs index 6810116cf5..82c10b56be 100644 --- a/runtime-sdk/src/types/transaction.rs +++ b/runtime-sdk/src/types/transaction.rs @@ -175,7 +175,8 @@ pub struct Fee { /// Maximum amount of gas paid for. #[cbor(optional)] pub gas: u64, - /// Maximum amount of emitted consensus messages paid for. + /// Maximum amount of emitted consensus messages paid for. Zero means that up to the maximum + /// number of per-batch messages can be emitted. #[cbor(optional)] pub consensus_messages: u32, } diff --git a/tests/e2e/evmtest.go b/tests/e2e/evmtest.go index 3349cf2cb7..e980056b2d 100644 --- a/tests/e2e/evmtest.go +++ b/tests/e2e/evmtest.go @@ -144,7 +144,6 @@ func evmCall(ctx context.Context, rtc client.RuntimeClient, e evm.V1, signer sig } } - txB.SetFeeConsensusMessages(1) tx := txB.SetFeeAmount(types.NewBaseUnits(*quantity.NewFromUint64(gasPrice * gasLimit), types.NativeDenomination)).GetTransaction() result, err := txgen.SignAndSubmitTxRaw(ctx, rtc, signer, *tx, gasLimit) if err != nil { diff --git a/tests/e2e/simple_consensus.go b/tests/e2e/simple_consensus.go index 07f0765f9f..c2ed38ca3c 100644 --- a/tests/e2e/simple_consensus.go +++ b/tests/e2e/simple_consensus.go @@ -295,7 +295,7 @@ func ConsensusDepositWithdrawalTest(sc *RuntimeScenario, log *logging.Logger, co amount := types.NewBaseUnits(*quantity.NewFromUint64(50_000), consDenomination) consensusAmount := quantity.NewFromUint64(50) tb := consAccounts.Deposit(&testing.Bob.Address, amount). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Alice.SigSpec, 0) _ = tb.AppendSign(ctx, testing.Alice.Signer) if err = tb.SubmitTx(ctx, nil); err != nil { @@ -328,7 +328,7 @@ func ConsensusDepositWithdrawalTest(sc *RuntimeScenario, log *logging.Logger, co consensusAmount = quantity.NewFromUint64(40) log.Info("bob depositing into runtime to alice") tb = consAccounts.Deposit(&testing.Alice.Address, amount). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Bob.SigSpec, 0) _ = tb.AppendSign(ctx, testing.Bob.Signer) if err = tb.SubmitTx(ctx, nil); err != nil { @@ -360,7 +360,7 @@ func ConsensusDepositWithdrawalTest(sc *RuntimeScenario, log *logging.Logger, co consensusAmount = quantity.NewFromUint64(25) log.Info("alice withdrawing to bob") tb = consAccounts.Withdraw(&testing.Bob.Address, amount). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Alice.SigSpec, 1) _ = tb.AppendSign(ctx, testing.Alice.Signer) if err = tb.SubmitTx(ctx, nil); err != nil { @@ -380,7 +380,7 @@ func ConsensusDepositWithdrawalTest(sc *RuntimeScenario, log *logging.Logger, co amount.Amount = *quantity.NewFromUint64(50_000) log.Info("charlie withdrawing") tb = consAccounts.Withdraw(&testing.Charlie.Address, amount). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Charlie.SigSpec, 0) _ = tb.AppendSign(ctx, testing.Charlie.Signer) if err = tb.SubmitTx(ctx, nil); err != nil { @@ -395,7 +395,7 @@ func ConsensusDepositWithdrawalTest(sc *RuntimeScenario, log *logging.Logger, co log.Info("alice withdrawing with invalid nonce") tb = consAccounts.Withdraw(&testing.Bob.Address, amount). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Alice.SigSpec, 1) _ = tb.AppendSign(ctx, testing.Alice.Signer) if err = tb.SubmitTx(ctx, nil); err != nil { @@ -436,7 +436,7 @@ func ConsensusDepositWithdrawalTest(sc *RuntimeScenario, log *logging.Logger, co log.Info("dave depositing (secp256k1)") amount.Amount = *quantity.NewFromUint64(50_000) tb = consAccounts.Deposit(&testing.Dave.Address, amount). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Dave.SigSpec, 0) _ = tb.AppendSign(ctx, testing.Dave.Signer) if err = tb.SubmitTx(ctx, nil); err != nil { @@ -496,7 +496,7 @@ func ConsensusDelegationTest(sc *RuntimeScenario, log *logging.Logger, conn *grp amount := types.NewBaseUnits(*quantity.NewFromUint64(10_000), consDenomination) consensusAmount := quantity.NewFromUint64(10) tb := consAccounts.Delegate(testing.Bob.Address, amount). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Alice.SigSpec, nonce) _ = tb.AppendSign(ctx, testing.Alice.Signer) if err = tb.SubmitTx(ctx, nil); err != nil { @@ -562,7 +562,7 @@ func ConsensusDelegationTest(sc *RuntimeScenario, log *logging.Logger, conn *grp amount = types.NewBaseUnits(*quantity.NewFromUint64(3_000), consDenomination) consensusAmount = quantity.NewFromUint64(3) tb = consAccounts.Delegate(testing.Alice.Address, amount). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Alice.SigSpec, nonce+1) _ = tb.AppendSign(ctx, testing.Alice.Signer) if err = tb.SubmitTx(ctx, nil); err != nil { @@ -590,14 +590,14 @@ func ConsensusDelegationTest(sc *RuntimeScenario, log *logging.Logger, conn *grp sharesA := quantity.NewFromUint64(1) consensusAmountA := quantity.NewFromUint64(1) tb = consAccounts.Undelegate(testing.Bob.Address, *sharesB). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Alice.SigSpec, nonce+2) _ = tb.AppendSign(ctx, testing.Alice.Signer) if err = tb.SubmitTx(ctx, nil); err != nil { return err } tb = consAccounts.Undelegate(testing.Alice.Address, *sharesA). - SetFeeConsensusMessages(1). + SetFeeGas(39). // Enough to emit 1 consensus message (max_batch_gas / max_messages = 10_000 / 256). AppendAuthSignature(testing.Alice.SigSpec, nonce+3) _ = tb.AppendSign(ctx, testing.Alice.Signer) if err = tb.SubmitTx(ctx, nil); err != nil {