From f4bf6556aadfa4fd68fad67af398c09e0984a438 Mon Sep 17 00:00:00 2001 From: Milosz Muszynski Date: Fri, 6 Sep 2024 14:05:31 +0200 Subject: [PATCH 1/3] rusk: verifying that deployment transactions have sufficient gas price --- rusk/CHANGELOG.md | 2 ++ rusk/default.config.toml | 1 + rusk/src/bin/config/chain.rs | 5 +++++ rusk/src/bin/main.rs | 1 + rusk/src/lib/node.rs | 1 + rusk/src/lib/node/rusk.rs | 28 ++++++++++++++++++++++++++-- 6 files changed, 36 insertions(+), 2 deletions(-) diff --git a/rusk/CHANGELOG.md b/rusk/CHANGELOG.md index 9f199491be..40b7d79493 100644 --- a/rusk/CHANGELOG.md +++ b/rusk/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Allow state transitions to be executed in parallel with queries [#970] - Change dependencies declarations enforce bytecheck [#1371] - Fixed tests passing incorrect arguments [#1371] +- Adjusted deployment charging [#2207] ### Added @@ -211,6 +212,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add linking between Rusk and Protobuff structs - Add build system that generates keys for circuits and caches them. +[#2207]: https://github.com/dusk-network/rusk/issues/2207 [#1884]: https://github.com/dusk-network/rusk/issues/1884 [#1882]: https://github.com/dusk-network/rusk/issues/1882 [#1675]: https://github.com/dusk-network/rusk/issues/1675 diff --git a/rusk/default.config.toml b/rusk/default.config.toml index c0dd6cf904..45b7d5baee 100644 --- a/rusk/default.config.toml +++ b/rusk/default.config.toml @@ -14,6 +14,7 @@ #generation_timeout = '3s' # Note: changing the gas per deploy byte parameter is equivalent to forking the chain. #gas_per_deploy_byte = 100 +#min_deployment_gas_price = 2000 [databroker] max_inv_entries = 100 diff --git a/rusk/src/bin/config/chain.rs b/rusk/src/bin/config/chain.rs index fff953373b..53dca0d586 100644 --- a/rusk/src/bin/config/chain.rs +++ b/rusk/src/bin/config/chain.rs @@ -27,6 +27,7 @@ pub(crate) struct ChainConfig { // NB: changing the gas_per_deploy_byte/block_gas_limit is equivalent to // forking the chain. gas_per_deploy_byte: Option, + min_deployment_gas_price: Option, block_gas_limit: Option, } @@ -79,6 +80,10 @@ impl ChainConfig { self.gas_per_deploy_byte } + pub(crate) fn min_deployment_gas_price(&self) -> Option { + self.min_deployment_gas_price + } + pub(crate) fn max_queue_size(&self) -> usize { self.max_queue_size.unwrap_or(10_000) } diff --git a/rusk/src/bin/main.rs b/rusk/src/bin/main.rs index 9a913b7408..f197884956 100644 --- a/rusk/src/bin/main.rs +++ b/rusk/src/bin/main.rs @@ -67,6 +67,7 @@ async fn main() -> Result<(), Box> { config.kadcast.chain_id(), config.chain.generation_timeout(), config.chain.gas_per_deploy_byte(), + config.chain.min_deployment_gas_price(), config.chain.block_gas_limit(), config.http.feeder_call_gas, _event_sender.clone(), diff --git a/rusk/src/lib/node.rs b/rusk/src/lib/node.rs index df4daeba3d..5a72d9a25a 100644 --- a/rusk/src/lib/node.rs +++ b/rusk/src/lib/node.rs @@ -45,6 +45,7 @@ pub struct Rusk { pub(crate) chain_id: u8, pub(crate) generation_timeout: Option, pub(crate) gas_per_deploy_byte: Option, + pub(crate) min_deployment_gas_price: Option, pub(crate) feeder_gas_limit: u64, pub(crate) block_gas_limit: u64, pub(crate) event_sender: broadcast::Sender, diff --git a/rusk/src/lib/node/rusk.rs b/rusk/src/lib/node/rusk.rs index 39e3095c1b..8ebf8a0150 100644 --- a/rusk/src/lib/node/rusk.rs +++ b/rusk/src/lib/node/rusk.rs @@ -50,13 +50,16 @@ pub static DUSK_KEY: LazyLock = LazyLock::new(|| { }); const DEFAULT_GAS_PER_DEPLOY_BYTE: u64 = 100; +const DEFAULT_MIN_DEPLOYMENT_GAS_PRICE: u64 = 2000; impl Rusk { + #[allow(clippy::too_many_arguments)] pub fn new>( dir: P, chain_id: u8, generation_timeout: Option, gas_per_deploy_byte: Option, + min_deployment_gas_price: Option, block_gas_limit: u64, feeder_gas_limit: u64, event_sender: broadcast::Sender, @@ -92,6 +95,7 @@ impl Rusk { chain_id, generation_timeout, gas_per_deploy_byte, + min_deployment_gas_price, feeder_gas_limit, event_sender, block_gas_limit, @@ -141,6 +145,7 @@ impl Rusk { &mut session, &unspent_tx.inner, self.gas_per_deploy_byte, + self.min_deployment_gas_price, ) { Ok(receipt) => { let gas_spent = receipt.gas_spent; @@ -159,6 +164,7 @@ impl Rusk { &mut session, &spent_tx.inner.inner, self.gas_per_deploy_byte, + self.min_deployment_gas_price, ); } @@ -244,6 +250,7 @@ impl Rusk { slashing, voters, self.gas_per_deploy_byte, + self.min_deployment_gas_price, ) .map(|(a, b, _, _)| (a, b)) } @@ -275,6 +282,7 @@ impl Rusk { slashing, voters, self.gas_per_deploy_byte, + self.min_deployment_gas_price, )?; if let Some(expected_verification) = consistency_check { @@ -467,6 +475,7 @@ fn accept( slashing: Vec, voters: Option<&[Voter]>, gas_per_deploy_byte: Option, + min_deployment_gas_price: Option, ) -> Result<( Vec, VerificationOutput, @@ -485,7 +494,12 @@ fn accept( for unspent_tx in txs { let tx = &unspent_tx.inner; - let receipt = execute(&mut session, tx, gas_per_deploy_byte)?; + let receipt = execute( + &mut session, + tx, + gas_per_deploy_byte, + min_deployment_gas_price, + )?; update_hasher(&mut event_hasher, &receipt.events); events.extend(receipt.events); @@ -592,7 +606,8 @@ fn contract_deploy( /// The following steps are performed: /// /// 1. Check if the transaction contains contract deployment data, and if so, -/// verifies if gas limit is enough for deployment. If gas limit is not +/// verifies if gas limit is enough for deployment and if the gas price is +/// sufficient for deployment. If either gas price or gas limit is not /// sufficient for deployment, transaction is discarded. /// /// 2. Call the "spend_and_execute" function on the transfer contract with @@ -625,12 +640,21 @@ fn execute( session: &mut Session, tx: &ProtocolTransaction, gas_per_deploy_byte: Option, + min_deployment_gas_price: Option, ) -> Result, ContractError>>, PiecrustError> { // Transaction will be discarded if it is a deployment transaction // with gas limit smaller than deploy charge. if let Some(deploy) = tx.deploy() { let deploy_charge = bytecode_charge(&deploy.bytecode, &gas_per_deploy_byte); + if tx.gas_price() + < min_deployment_gas_price + .unwrap_or(DEFAULT_MIN_DEPLOYMENT_GAS_PRICE) + { + return Err(PiecrustError::Panic( + "gas price too low to deploy".into(), + )); + } if tx.gas_limit() < deploy_charge { return Err(PiecrustError::Panic( "not enough gas to deploy".into(), From 7a9a06e2eb6c42acf0076c93bdb8bcb7f2e55bf7 Mon Sep 17 00:00:00 2001 From: Milosz Muszynski Date: Fri, 6 Sep 2024 14:06:52 +0200 Subject: [PATCH 2/3] rusk: test adjustments for deployment that requires sufficient gas price --- rusk/tests/common/state.rs | 14 ++++-- rusk/tests/config/contract_deployment.toml | 2 +- rusk/tests/services/contract_deployment.rs | 50 ++++++++++++++-------- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/rusk/tests/common/state.rs b/rusk/tests/common/state.rs index 153be2ae5f..a4d1df42e5 100644 --- a/rusk/tests/common/state.rs +++ b/rusk/tests/common/state.rs @@ -44,9 +44,17 @@ pub fn new_state>( let (sender, _) = broadcast::channel(10); - let rusk = - Rusk::new(dir, CHAIN_ID, None, None, block_gas_limit, u64::MAX, sender) - .expect("Instantiating rusk should succeed"); + let rusk = Rusk::new( + dir, + CHAIN_ID, + None, + None, + None, + block_gas_limit, + u64::MAX, + sender, + ) + .expect("Instantiating rusk should succeed"); assert_eq!( commit_id, diff --git a/rusk/tests/config/contract_deployment.toml b/rusk/tests/config/contract_deployment.toml index 1d61cec0e9..c172c40afc 100644 --- a/rusk/tests/config/contract_deployment.toml +++ b/rusk/tests/config/contract_deployment.toml @@ -1,4 +1,4 @@ [[phoenix_balance]] address = "ivmscertKgRyX8wNMJJsQcSVEyPsfSMUQXSAgeAPQXsndqFq9Pmknzhm61QvcEEdxPaGgxDS4RHpb6KKccrnSKN" seed = 57005 -notes = [10_000_000_000] +notes = [1_000_000_000_000] diff --git a/rusk/tests/services/contract_deployment.rs b/rusk/tests/services/contract_deployment.rs index 0aa17d5751..a0e225060e 100644 --- a/rusk/tests/services/contract_deployment.rs +++ b/rusk/tests/services/contract_deployment.rs @@ -31,9 +31,8 @@ const BLOCK_HEIGHT: u64 = 1; const BLOCK_GAS_LIMIT: u64 = 1_000_000_000_000; const GAS_LIMIT: u64 = 200_000_000; const GAS_LIMIT_NOT_ENOUGH_TO_SPEND: u64 = 10_000_000; -const GAS_LIMIT_NOT_ENOUGH_TO_SPEND_AND_DEPLOY: u64 = 11_000_000; const GAS_LIMIT_NOT_ENOUGH_TO_DEPLOY: u64 = 1_200_000; -const GAS_PRICE: u64 = 2; +const GAS_PRICE: u64 = 2000; const POINT_LIMIT: u64 = 0x10000000; const SENDER_INDEX: u8 = 0; @@ -81,7 +80,7 @@ fn initial_state>(dir: P, deploy_bob: bool) -> Result { bob_bytecode, ContractData::builder() .owner(OWNER) - .constructor_arg(&BOB_INIT_VALUE) + .init_arg(&BOB_INIT_VALUE) .contract_id(gen_contract_id( &bob_bytecode, 0u64, @@ -96,9 +95,17 @@ fn initial_state>(dir: P, deploy_bob: bool) -> Result { let (sender, _) = broadcast::channel(10); - let rusk = - Rusk::new(dir, CHAIN_ID, None, None, BLOCK_GAS_LIMIT, u64::MAX, sender) - .expect("Instantiating rusk should succeed"); + let rusk = Rusk::new( + dir, + CHAIN_ID, + None, + None, + None, + BLOCK_GAS_LIMIT, + u64::MAX, + sender, + ) + .expect("Instantiating rusk should succeed"); Ok(rusk) } @@ -115,6 +122,7 @@ fn make_and_execute_transaction_deploy( init_value: u8, should_fail: bool, should_discard: bool, + gas_price: u64, ) { let mut rng = StdRng::seed_from_u64(0xcafe); @@ -126,7 +134,7 @@ fn make_and_execute_transaction_deploy( &mut rng, SENDER_INDEX, gas_limit, - GAS_PRICE, + gas_price, 0u64, TransactionData::Deploy(ContractDeploy { bytecode: ContractBytecode { @@ -282,6 +290,7 @@ pub async fn contract_deploy() { BOB_INIT_VALUE, false, false, + GAS_PRICE, ); let after_balance = f.wallet_balance(); f.assert_bob_contract_is_deployed(); @@ -307,6 +316,7 @@ pub async fn contract_already_deployed() { BOB_INIT_VALUE, true, false, + GAS_PRICE, ); let after_balance = f.wallet_balance(); let funds_spent = before_balance - after_balance; @@ -334,6 +344,7 @@ pub async fn contract_deploy_corrupted_bytecode() { BOB_INIT_VALUE, true, false, + GAS_PRICE, ); let after_balance = f.wallet_balance(); let funds_spent = before_balance - after_balance; @@ -360,6 +371,7 @@ pub async fn contract_deploy_charge() { BOB_INIT_VALUE, false, false, + GAS_PRICE, ); let after_bob_balance = f.wallet_balance(); make_and_execute_transaction_deploy( @@ -370,6 +382,7 @@ pub async fn contract_deploy_charge() { 0, false, false, + GAS_PRICE, ); let after_license_balance = f.wallet_balance(); let bob_deployment_cost = before_balance - after_bob_balance; @@ -396,6 +409,7 @@ pub async fn contract_deploy_not_enough_to_spend() { BOB_INIT_VALUE, false, true, + GAS_PRICE, ); let after_balance = f.wallet_balance(); f.assert_bob_contract_is_not_deployed(); @@ -403,12 +417,11 @@ pub async fn contract_deploy_not_enough_to_spend() { assert_eq!(funds_spent, 0); } -/// We deploy a contract with insufficient gas limit. -/// The limit is such that it is enough to spend but not enough to deploy. -/// Transaction won't be discarded and all limit will be spent by the wallet. -/// Wallet will spend GAS_LIMIT_NOT_ENOUGH_TO_DEPLOY x GAS_PRICE of funds. +/// We deploy a contract with insufficient gas price. +/// Transaction will be discarded. #[tokio::test(flavor = "multi_thread")] -pub async fn contract_deploy_not_enough_to_spend_and_deploy() { +pub async fn contract_deploy_gas_price_too_low() { + const LOW_GAS_PRICE: u64 = 10; logger(); let f = Fixture::build(false); @@ -418,25 +431,23 @@ pub async fn contract_deploy_not_enough_to_spend_and_deploy() { &f.rusk, &f.wallet, f.bob_bytecode.clone(), - GAS_LIMIT_NOT_ENOUGH_TO_SPEND_AND_DEPLOY, + GAS_LIMIT, BOB_INIT_VALUE, - true, false, + true, + LOW_GAS_PRICE, ); let after_balance = f.wallet_balance(); f.assert_bob_contract_is_not_deployed(); let funds_spent = before_balance - after_balance; - assert_eq!( - funds_spent, - GAS_LIMIT_NOT_ENOUGH_TO_SPEND_AND_DEPLOY * GAS_PRICE - ); + assert_eq!(funds_spent, 0); } /// We deploy a contract with insufficient gas limit. /// The limit is such that it is not enough to deploy. /// Transaction will be discarded and no funds will be spent by the wallet. #[tokio::test(flavor = "multi_thread")] -pub async fn contract_deploy_not_enough_to_deploy() { +pub async fn contract_deploy_gas_limit_too_low() { logger(); let f = Fixture::build(false); @@ -450,6 +461,7 @@ pub async fn contract_deploy_not_enough_to_deploy() { BOB_INIT_VALUE, false, true, + GAS_PRICE, ); let after_balance = f.wallet_balance(); f.assert_bob_contract_is_not_deployed(); From 104f9fcea16efcd7eb513107c133e09a125c1422 Mon Sep 17 00:00:00 2001 From: Milosz Muszynski Date: Fri, 6 Sep 2024 14:08:14 +0200 Subject: [PATCH 3/3] rusk: test adjustments for deployment and piecrust changes --- rusk/tests/services/owner_calls.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/rusk/tests/services/owner_calls.rs b/rusk/tests/services/owner_calls.rs index d3a932bdc5..f784185299 100644 --- a/rusk/tests/services/owner_calls.rs +++ b/rusk/tests/services/owner_calls.rs @@ -64,7 +64,7 @@ fn initial_state>( bob_bytecode, ContractData::builder() .owner(owner.as_ref()) - .constructor_arg(&BOB_INIT_VALUE) + .init_arg(&BOB_INIT_VALUE) .contract_id(gen_contract_id(&bob_bytecode, 0u64, owner)), POINT_LIMIT, ) @@ -74,9 +74,17 @@ fn initial_state>( let (sender, _) = broadcast::channel(10); - let rusk = - Rusk::new(dir, CHAIN_ID, None, None, BLOCK_GAS_LIMIT, u64::MAX, sender) - .expect("Instantiating rusk should succeed"); + let rusk = Rusk::new( + dir, + CHAIN_ID, + None, + None, + None, + BLOCK_GAS_LIMIT, + u64::MAX, + sender, + ) + .expect("Instantiating rusk should succeed"); Ok(rusk) }