From 7de05de8b22f24fec5cbe08d792c940f61c8fd5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Papierski?= Date: Fri, 17 Jan 2025 15:00:37 +0100 Subject: [PATCH 1/4] Bump rust toolchain This apparently fixes some rust-analyzer issues I was encountering without introducing too many code fixes related to clippy/fmt etc. --- .../reactor/main_reactor/tests/transactions.rs | 16 ++++++++-------- rust-toolchain.toml | 2 +- smart_contracts/sdk/src/lib.rs | 10 ---------- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/node/src/reactor/main_reactor/tests/transactions.rs b/node/src/reactor/main_reactor/tests/transactions.rs index f06fa350ae..1409fb2aed 100644 --- a/node/src/reactor/main_reactor/tests/transactions.rs +++ b/node/src/reactor/main_reactor/tests/transactions.rs @@ -98,7 +98,7 @@ async fn send_wasm_transaction( TransactionV1Builder::new_session( false, module_bytes, - casper_types::TransactionRuntimeParams::VmCasperV1, + TransactionRuntimeParams::VmCasperV1, ) .with_chain_name(chain_name) .with_pricing_mode(pricing) @@ -1769,7 +1769,7 @@ async fn only_refunds_are_burnt_no_fee_custom_payment() { TransactionV1Builder::new_session( false, module_bytes, - casper_types::TransactionRuntimeParams::VmCasperV1, + TransactionRuntimeParams::VmCasperV1, ) .with_chain_name(CHAIN_NAME) .with_pricing_mode(PricingMode::PaymentLimited { @@ -2434,7 +2434,7 @@ fn invalid_wasm_txn(initiator: Arc, pricing_mode: PricingMode) -> Tra TransactionV1Builder::new_session( false, module_bytes, - casper_types::TransactionRuntimeParams::VmCasperV1, + TransactionRuntimeParams::VmCasperV1, ) .with_chain_name(CHAIN_NAME) .with_pricing_mode(pricing_mode) @@ -3141,7 +3141,7 @@ async fn insufficient_funds_transfer_from_purse() { TransactionV1Builder::new_session( false, module_bytes, - casper_types::TransactionRuntimeParams::VmCasperV1, + TransactionRuntimeParams::VmCasperV1, ) .with_runtime_args(runtime_args! { "destination" => purse_name, "amount" => U512::zero() }) .with_chain_name(CHAIN_NAME) @@ -3267,7 +3267,7 @@ async fn charge_when_session_code_succeeds() { TransactionV1Builder::new_session( false, module_bytes, - casper_types::TransactionRuntimeParams::VmCasperV1, + TransactionRuntimeParams::VmCasperV1, ) .with_runtime_args(runtime_args! { ARG_TARGET => CHARLIE_PUBLIC_KEY.to_account_hash(), @@ -3338,7 +3338,7 @@ async fn charge_when_session_code_fails_with_user_error() { TransactionV1Builder::new_session( false, module_bytes, - casper_types::TransactionRuntimeParams::VmCasperV1, + TransactionRuntimeParams::VmCasperV1, ) .with_chain_name(CHAIN_NAME) .with_initiator_addr(BOB_PUBLIC_KEY.clone()) @@ -3406,7 +3406,7 @@ async fn charge_when_session_code_runs_out_of_gas() { TransactionV1Builder::new_session( false, module_bytes, - casper_types::TransactionRuntimeParams::VmCasperV1, + TransactionRuntimeParams::VmCasperV1, ) .with_chain_name(CHAIN_NAME) .with_initiator_addr(BOB_PUBLIC_KEY.clone()) @@ -3732,7 +3732,7 @@ async fn out_of_gas_txn_does_not_produce_effects() { TransactionV1Builder::new_session( false, module_bytes, - casper_types::TransactionRuntimeParams::VmCasperV1, + TransactionRuntimeParams::VmCasperV1, ) .with_chain_name(CHAIN_NAME) .with_initiator_addr(BOB_PUBLIC_KEY.clone()) diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 6f14058b2e..51985806fc 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,2 +1,2 @@ [toolchain] -channel = "1.77.2" +channel = "1.78.0" diff --git a/smart_contracts/sdk/src/lib.rs b/smart_contracts/sdk/src/lib.rs index b758453770..8b56861fc0 100644 --- a/smart_contracts/sdk/src/lib.rs +++ b/smart_contracts/sdk/src/lib.rs @@ -342,16 +342,6 @@ impl<'a, T: ContractRef> ContractBuilder<'a, T> { #[cfg(test)] mod tests { - use super::*; - - #[allow(dead_code)] - struct MyContract; - - #[derive(BorshSerialize)] - struct DoSomethingArg { - foo: u64, - } - // impl ToCallData for DoSomethingArg { // const SELECTOR: Selector = Selector::new(1); // type Return<'a> = (); From 172c024853c4a99bb777bd5b5397e59a44e20cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Papierski?= Date: Fri, 17 Jan 2025 16:49:06 +0100 Subject: [PATCH 2/4] Regression test --- Cargo.lock | 8 ++ node/src/components/contract_runtime/tests.rs | 4 +- .../main_reactor/tests/transactions.rs | 79 ++++++++++++++++++- .../test/gh-5058-regression/Cargo.toml | 15 ++++ .../test/gh-5058-regression/src/main.rs | 61 ++++++++++++++ 5 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 smart_contracts/contracts/test/gh-5058-regression/Cargo.toml create mode 100644 smart_contracts/contracts/test/gh-5058-regression/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index b8b53c15c6..cf65900377 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2827,6 +2827,14 @@ dependencies = [ "casper-types", ] +[[package]] +name = "gh-5058-regression" +version = "0.1.0" +dependencies = [ + "casper-contract", + "casper-types", +] + [[package]] name = "gimli" version = "0.26.2" diff --git a/node/src/components/contract_runtime/tests.rs b/node/src/components/contract_runtime/tests.rs index fc4d53bd36..ac85fe98f4 100644 --- a/node/src/components/contract_runtime/tests.rs +++ b/node/src/components/contract_runtime/tests.rs @@ -1,6 +1,7 @@ -use std::{collections::BTreeMap, iter, sync::Arc, time::Duration}; +use std::{collections::BTreeMap, fs, iter, path::PathBuf, sync::Arc, time::Duration}; use derive_more::{Display, From}; +use once_cell::sync::Lazy; use prometheus::Registry; use rand::RngCore; use serde::Serialize; @@ -11,6 +12,7 @@ use casper_types::{ ExecutableDeployItem, PublicKey, SecretKey, TimeDiff, Timestamp, Transaction, TransactionConfig, MINT_LANE_ID, U512, }; +use tests::testing::LARGE_WASM_LANE_ID; use super::*; use crate::{ diff --git a/node/src/reactor/main_reactor/tests/transactions.rs b/node/src/reactor/main_reactor/tests/transactions.rs index 1409fb2aed..a9d0c8243b 100644 --- a/node/src/reactor/main_reactor/tests/transactions.rs +++ b/node/src/reactor/main_reactor/tests/transactions.rs @@ -9,7 +9,8 @@ use casper_types::{ addressable_entity::NamedKeyAddr, runtime_args, system::mint::{ARG_AMOUNT, ARG_TARGET}, - AddressableEntity, Digest, EntityAddr, ExecutionInfo, TransactionRuntimeParams, + AddressableEntity, Digest, EntityAddr, ExecutableDeployItem, ExecutionInfo, + TransactionRuntimeParams, }; use once_cell::sync::Lazy; @@ -3951,3 +3952,79 @@ async fn gas_holds_accumulate_for_multiple_transactions_in_the_same_block() { "Holds should have expired." ); } + +#[tokio::test] +async fn gh_5058_regression_custom_payment_with_deploy_variant_works() { + let config = SingleTransactionTestCase::default_test_config() + .with_pricing_handling(PricingHandling::Classic) + .with_refund_handling(RefundHandling::NoRefund) + .with_fee_handling(FeeHandling::NoFee); + + let mut test = SingleTransactionTestCase::new( + ALICE_SECRET_KEY.clone(), + BOB_SECRET_KEY.clone(), + CHARLIE_SECRET_KEY.clone(), + Some(config), + ) + .await; + + test.fixture + .run_until_consensus_in_era(ERA_ONE, ONE_MIN) + .await; + + // This WASM creates named key called "new_key". Then it would loop endlessly trying to write a + // value to storage. Eventually it will run out of gas and it should exit causing a revert. + let base_path = RESOURCES_PATH + .parent() + .unwrap() + .join("target") + .join("wasm32-unknown-unknown") + .join("release"); + + let payment_amount = U512::from(1_000_000u64); + + let txn = { + let timestamp = Timestamp::now(); + let ttl = TimeDiff::from_seconds(100); + let gas_price = 1; + let chain_name = test.chainspec().network_config.name.clone(); + + let payment = ExecutableDeployItem::ModuleBytes { + module_bytes: std::fs::read(base_path.join("gh_5058_regression.wasm")) + .unwrap() + .into(), + args: runtime_args! { + "amount" => payment_amount, + "this_is_payment" => true, + }, + }; + + let session = ExecutableDeployItem::ModuleBytes { + module_bytes: std::fs::read(base_path.join("do_nothing.wasm")) + .unwrap() + .into(), + args: runtime_args! { + "this_is_session" => true, + }, + }; + + Transaction::Deploy(Deploy::new_signed( + timestamp, + ttl, + gas_price, + vec![], + chain_name.clone(), + payment, + session, + &ALICE_SECRET_KEY, + Some(ALICE_PUBLIC_KEY.clone()), + )) + }; + + let acct = get_balance(&mut test.fixture, &ALICE_PUBLIC_KEY, None, true); + assert!(acct.total_balance().cloned().unwrap() >= payment_amount); + + let (_txn_hash, _block_height, exec_result) = test.send_transaction(txn).await; + + assert_eq!(exec_result.error_message(), None); +} diff --git a/smart_contracts/contracts/test/gh-5058-regression/Cargo.toml b/smart_contracts/contracts/test/gh-5058-regression/Cargo.toml new file mode 100644 index 0000000000..3551e5aeec --- /dev/null +++ b/smart_contracts/contracts/test/gh-5058-regression/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "gh-5058-regression" +version = "0.1.0" +edition = "2021" + +[[bin]] +name = "gh_5058_regression" +path = "src/main.rs" +bench = false +doctest = false +test = false + +[dependencies] +casper-contract = { path = "../../../contract" } +casper-types = { path = "../../../../types" } diff --git a/smart_contracts/contracts/test/gh-5058-regression/src/main.rs b/smart_contracts/contracts/test/gh-5058-regression/src/main.rs new file mode 100644 index 0000000000..7fce5f502a --- /dev/null +++ b/smart_contracts/contracts/test/gh-5058-regression/src/main.rs @@ -0,0 +1,61 @@ +#![no_main] +#![no_std] + +extern crate alloc; + +use casper_contract::{ + contract_api::{account, runtime, system}, + unwrap_or_revert::UnwrapOrRevert, +}; + +use casper_types::{ + runtime_args, system::handle_payment, ApiError, Phase, RuntimeArgs, URef, U512, +}; + +const ARG_AMOUNT: &str = "amount"; + +#[repr(u16)] +enum Error { + InvalidPhase, +} + +impl From for ApiError { + fn from(e: Error) -> Self { + ApiError::User(e as u16) + } +} + +fn get_payment_purse() -> URef { + runtime::call_contract( + system::get_handle_payment(), + handle_payment::METHOD_GET_PAYMENT_PURSE, + RuntimeArgs::default(), + ) +} + +fn set_refund_purse(new_refund_purse: URef) { + let args = runtime_args! { + handle_payment::ARG_PURSE => new_refund_purse, + }; + + runtime::call_contract( + system::get_handle_payment(), + handle_payment::METHOD_SET_REFUND_PURSE, + args, + ) +} + +#[no_mangle] +pub extern "C" fn call() { + if runtime::get_phase() != Phase::Payment { + runtime::revert(Error::InvalidPhase); + } + + let amount: U512 = runtime::get_named_arg(ARG_AMOUNT); + let payment_purse = get_payment_purse(); + set_refund_purse(account::get_main_purse()); + + // transfer amount from named purse to payment purse, which will be used to pay for execution + system::transfer_from_purse_to_purse(account::get_main_purse(), payment_purse, amount, None) + .unwrap_or_revert(); +} From 2e9649d7756b8015c503d009c1089443783cc746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Papierski?= Date: Fri, 17 Jan 2025 17:00:47 +0100 Subject: [PATCH 3/4] Fix custom payment under deploy tx variant This change will take payment field if detected transaction is of "Deploy" variant, and changes the handle payment to properly clear the named key. --- .../components/contract_runtime/operations.rs | 6 ++- .../src/types/transaction/meta_transaction.rs | 48 +++++++++++++++++++ .../src/data_access_layer/handle_refund.rs | 8 ++-- 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/node/src/components/contract_runtime/operations.rs b/node/src/components/contract_runtime/operations.rs index f22ce642cd..be50cf90c3 100644 --- a/node/src/components/contract_runtime/operations.rs +++ b/node/src/components/contract_runtime/operations.rs @@ -308,11 +308,13 @@ pub fn execute_finalized_block( // in a custom way. If anything goes wrong, penalize the sender, do not execute let custom_payment_gas_limit = Gas::new(chainspec.transaction_config.native_transfer_minimum_motes * 5); - let session_input_data = transaction.to_session_input_data(); + + let payment_input_data = transaction.to_payment_input_data(); + let pay_result = match WasmV1Request::new_custom_payment( BlockInfo::new(state_root_hash, block_time, parent_block_hash, block_height), custom_payment_gas_limit, - &session_input_data, + &payment_input_data, ) { Ok(mut pay_request) => { // We'll send a hint to the custom payment logic on the amount diff --git a/node/src/types/transaction/meta_transaction.rs b/node/src/types/transaction/meta_transaction.rs index 702c0fef19..3858d66650 100644 --- a/node/src/types/transaction/meta_transaction.rs +++ b/node/src/types/transaction/meta_transaction.rs @@ -284,6 +284,54 @@ impl MetaTransaction { } } + /// Returns the `SessionInputData` for a payment code if present. + pub fn to_payment_input_data(&self) -> SessionInputData { + match self { + MetaTransaction::Deploy(meta_deploy) => { + let initiator_addr = + InitiatorAddr::PublicKey(meta_deploy.deploy().account().clone()); + let is_standard_payment = matches!(meta_deploy.deploy().payment(), ExecutableDeployItem::ModuleBytes { module_bytes, .. } if module_bytes.is_empty()); + let deploy = meta_deploy.deploy(); + let data = SessionDataDeploy::new( + deploy.hash(), + deploy.payment(), + initiator_addr, + self.signers().clone(), + is_standard_payment, + ); + SessionInputData::DeploySessionData { data } + } + MetaTransaction::V1(v1) => { + let initiator_addr = v1.initiator_addr().clone(); + + let is_standard_payment = if let PricingMode::PaymentLimited { + standard_payment, + .. + } = v1.pricing_mode() + { + *standard_payment + } else { + true + }; + + // Under V1 transaction we don't have a separate payment code, and custom payment is + // executed as session code with a phase set to Payment. + let data = SessionDataV1::new( + v1.args().as_named().expect("V1 wasm args should be named and validated at the transaction acceptor level"), + v1.target(), + v1.entry_point(), + v1.lane_id() == INSTALL_UPGRADE_LANE_ID, + v1.hash(), + v1.pricing_mode(), + initiator_addr, + self.signers().clone(), + is_standard_payment, + ); + SessionInputData::SessionDataV1 { data } + } + } + } + /// Size estimate. pub fn size_estimate(&self) -> usize { match self { diff --git a/storage/src/data_access_layer/handle_refund.rs b/storage/src/data_access_layer/handle_refund.rs index 9b022b70f2..e9fbe567d8 100644 --- a/storage/src/data_access_layer/handle_refund.rs +++ b/storage/src/data_access_layer/handle_refund.rs @@ -83,12 +83,14 @@ impl HandleRefundMode { /// Returns the appropriate phase for the mode. pub fn phase(&self) -> Phase { match self { - HandleRefundMode::ClearRefundPurse - | HandleRefundMode::Burn { .. } + HandleRefundMode::Burn { .. } | HandleRefundMode::Refund { .. } | HandleRefundMode::CustomHold { .. } | HandleRefundMode::RefundAmount { .. } => Phase::FinalizePayment, - HandleRefundMode::SetRefundPurse { .. } => Phase::Payment, + + HandleRefundMode::ClearRefundPurse | HandleRefundMode::SetRefundPurse { .. } => { + Phase::Payment + } } } } From e23de8a1fdefd4d6a428a24c04516907ab91f6df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Papierski?= Date: Fri, 17 Jan 2025 17:21:12 +0100 Subject: [PATCH 4/4] Fix clippy issues --- node/src/components/contract_runtime/tests.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/node/src/components/contract_runtime/tests.rs b/node/src/components/contract_runtime/tests.rs index ac85fe98f4..fc4d53bd36 100644 --- a/node/src/components/contract_runtime/tests.rs +++ b/node/src/components/contract_runtime/tests.rs @@ -1,7 +1,6 @@ -use std::{collections::BTreeMap, fs, iter, path::PathBuf, sync::Arc, time::Duration}; +use std::{collections::BTreeMap, iter, sync::Arc, time::Duration}; use derive_more::{Display, From}; -use once_cell::sync::Lazy; use prometheus::Registry; use rand::RngCore; use serde::Serialize; @@ -12,7 +11,6 @@ use casper_types::{ ExecutableDeployItem, PublicKey, SecretKey, TimeDiff, Timestamp, Transaction, TransactionConfig, MINT_LANE_ID, U512, }; -use tests::testing::LARGE_WASM_LANE_ID; use super::*; use crate::{