From 0258bce95c0ec0cd2b412e9e375af130dc641c25 Mon Sep 17 00:00:00 2001 From: Brandon Kite Date: Mon, 20 Nov 2023 10:43:55 -0800 Subject: [PATCH 01/14] rough sketch of token contract --- crates/chain-config/src/lib.rs | 47 +++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/crates/chain-config/src/lib.rs b/crates/chain-config/src/lib.rs index 5cd1755609d..980b98889ce 100644 --- a/crates/chain-config/src/lib.rs +++ b/crates/chain-config/src/lib.rs @@ -8,7 +8,21 @@ mod genesis; mod serialization; pub use config::*; -use fuel_core_types::fuel_vm::SecretKey; +use fuel_core_types::{ + fuel_asm::{ + op, + GTFArgs, + RegId, + }, + fuel_tx::{ + Address, + OutputRepr, + }, + fuel_vm::{ + CallFrame, + SecretKey, + }, +}; pub use genesis::GenesisCommitment; /// A default secret key to use for testing purposes only @@ -25,3 +39,34 @@ pub fn default_consensus_dev_key() -> SecretKey { ]; SecretKey::try_from(bytes.as_slice()).expect("valid key") } + +pub fn generate_fee_collection_contract(address: Address) -> Vec { + let asm = vec![ + // // obtain the output type of tx.output[0] + // op::gtf(0x10, GTFArgs::OutputType, 0), + // // store the expected output type for a variable output in the next register. + // op::movi(0x11, OutputRepr::Variable as u32), + // // panic if tx.outputs[0].type != OutputType.Variable + // op::eq(0x12, 0x11, 0x10), + // op::jnzf(0x12, RegId::ZERO, 1), + // op::rvrt(1), + // Pointer to AssetID memory address in call frame param a + op::addi(0x10, RegId::FP, CallFrame::a_offset() as u16), + // pointer to the withdrawal address embedded after the contract bytecode + op::addi( + 0x11, + RegId::IS, + 7, // update when number of opcodes changes + ), + // get the balance of asset ID in the contract + op::bal(0x11, 0x10, RegId::FP), + // if balance > 0, withdraw + op::eq(0x12, 0x11, RegId::ZERO), + op::jnzf(0x12, RegId::ZERO, 1), + op::tro(0, 0, 0x11, 0x10), + // return + op::ret(RegId::ONE), + ]; + + Default::default() +} From 56ac50eea136acf5208fd342e4fe6bba3c893895 Mon Sep 17 00:00:00 2001 From: Brandon Kite Date: Mon, 20 Nov 2023 10:55:16 -0800 Subject: [PATCH 02/14] add some todos for handoff --- crates/chain-config/src/lib.rs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/chain-config/src/lib.rs b/crates/chain-config/src/lib.rs index 980b98889ce..d90a0d796ca 100644 --- a/crates/chain-config/src/lib.rs +++ b/crates/chain-config/src/lib.rs @@ -41,15 +41,20 @@ pub fn default_consensus_dev_key() -> SecretKey { } pub fn generate_fee_collection_contract(address: Address) -> Vec { + // TODO: Tests to write for this contract: + // 1. Happy path withdrawal case for block producer + // Deploy the contract for the block producer's address + // Run some blocks and accumulate fees + // Attempt to withdraw the collected fees to the BP's address + // (note that currently we allow anyone to initiate this withdrawal) + // 2. Unhappy case where tx doesn't have the expected variable output set + // 3. Edge case, withdrawal is attempted when there are no fees collected (shouldn't revert, but the BP's balance should be the same) + + // TODO: setup cli interface for generating this contract + // This should be accessible to the fuel-core CLI. + // ie. something like `fuel-core generate-fee-contract ` -> + let asm = vec![ - // // obtain the output type of tx.output[0] - // op::gtf(0x10, GTFArgs::OutputType, 0), - // // store the expected output type for a variable output in the next register. - // op::movi(0x11, OutputRepr::Variable as u32), - // // panic if tx.outputs[0].type != OutputType.Variable - // op::eq(0x12, 0x11, 0x10), - // op::jnzf(0x12, RegId::ZERO, 1), - // op::rvrt(1), // Pointer to AssetID memory address in call frame param a op::addi(0x10, RegId::FP, CallFrame::a_offset() as u16), // pointer to the withdrawal address embedded after the contract bytecode @@ -63,10 +68,15 @@ pub fn generate_fee_collection_contract(address: Address) -> Vec { // if balance > 0, withdraw op::eq(0x12, 0x11, RegId::ZERO), op::jnzf(0x12, RegId::ZERO, 1), + // todo: point $rA to the location of the withdrawal address in memory (after return) op::tro(0, 0, 0x11, 0x10), // return op::ret(RegId::ONE), ]; - Default::default() + let mut asm_bytes: Vec = asm.into_iter().collect(); + // append withdrawal address at the end of the contract bytecode. + asm_bytes.extend_from_slice(address.as_slice()); + + asm_bytes } From 662c4fa419432eaf070f79a40dd0a128e3d60785 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Tue, 21 Nov 2023 20:29:56 +0200 Subject: [PATCH 03/14] Add CLI tool --- Cargo.lock | 3 ++ bin/fuel-core/Cargo.toml | 3 ++ bin/fuel-core/src/cli.rs | 3 ++ bin/fuel-core/src/cli/fee_contract.rs | 40 +++++++++++++++++++++++++++ crates/chain-config/src/lib.rs | 8 ++---- 5 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 bin/fuel-core/src/cli/fee_contract.rs diff --git a/Cargo.lock b/Cargo.lock index f349dc2f247..3d0bdccbade 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2831,6 +2831,9 @@ dependencies = [ "dirs 4.0.0", "dotenvy", "fuel-core", + "fuel-core-chain-config", + "fuel-core-types", + "hex", "humantime", "lazy_static", "pyroscope", diff --git a/bin/fuel-core/Cargo.toml b/bin/fuel-core/Cargo.toml index dbbd6cf9ed7..aeb579fdfa4 100644 --- a/bin/fuel-core/Cargo.toml +++ b/bin/fuel-core/Cargo.toml @@ -22,6 +22,9 @@ const_format = { version = "0.2", optional = true } dirs = "4.0" dotenvy = { version = "0.15", optional = true } fuel-core = { workspace = true } +fuel-core-types = { workspace = true } +fuel-core-chain-config = { workspace = true } +hex = "0.4" humantime = "2.1" lazy_static = { workspace = true } pyroscope = "0.5" diff --git a/bin/fuel-core/src/cli.rs b/bin/fuel-core/src/cli.rs index bee7d90c1c1..b3ba51c7de9 100644 --- a/bin/fuel-core/src/cli.rs +++ b/bin/fuel-core/src/cli.rs @@ -18,6 +18,7 @@ lazy_static::lazy_static! { pub static ref DEFAULT_DB_PATH: PathBuf = dirs::home_dir().unwrap().join(".fuel").join("db"); } +pub mod fee_contract; pub mod run; pub mod snapshot; @@ -38,6 +39,7 @@ pub struct Opt { pub enum Fuel { Run(run::Command), Snapshot(snapshot::Command), + GenerateFeeContract(fee_contract::Command), } pub const LOG_FILTER: &str = "RUST_LOG"; @@ -117,6 +119,7 @@ pub async fn run_cli() -> anyhow::Result<()> { Ok(opt) => match opt.command { Fuel::Run(command) => run::exec(command).await, Fuel::Snapshot(command) => snapshot::exec(command).await, + Fuel::GenerateFeeContract(command) => fee_contract::exec(command).await, }, Err(e) => { // Prints the error and exits. diff --git a/bin/fuel-core/src/cli/fee_contract.rs b/bin/fuel-core/src/cli/fee_contract.rs new file mode 100644 index 00000000000..ccf21492d67 --- /dev/null +++ b/bin/fuel-core/src/cli/fee_contract.rs @@ -0,0 +1,40 @@ +use clap::Parser; +use fuel_core_chain_config::generate_fee_collection_contract; +use fuel_core_types::fuel_tx::Address; +use std::{ + fs::OpenOptions, + io::Write, + path::PathBuf, +}; + +#[derive(Debug, Parser)] +pub struct Command { + /// Address to withdraw fees to + withdrawal_address: Address, + /// Output file. If not provided, will print hex representation to stdout. + #[clap(short, long)] + output: Option, + /// Overwrite output file if it exists. No effect if `output` is not provided. + #[clap(short, long)] + force: bool, +} + +pub async fn exec(cmd: Command) -> anyhow::Result<()> { + let contract = generate_fee_collection_contract(cmd.withdrawal_address); + + if let Some(output) = cmd.output.as_ref() { + let mut open_opt = OpenOptions::new(); + if cmd.force { + open_opt.create(true).write(true).truncate(true); + } else { + open_opt.create_new(true).write(true); + } + + let mut file = open_opt.open(output)?; + file.write_all(&contract)?; + } else { + println!("{}", hex::encode(contract)); + } + + Ok(()) +} diff --git a/crates/chain-config/src/lib.rs b/crates/chain-config/src/lib.rs index d90a0d796ca..c472adf21ef 100644 --- a/crates/chain-config/src/lib.rs +++ b/crates/chain-config/src/lib.rs @@ -11,13 +11,9 @@ pub use config::*; use fuel_core_types::{ fuel_asm::{ op, - GTFArgs, RegId, }, - fuel_tx::{ - Address, - OutputRepr, - }, + fuel_tx::Address, fuel_vm::{ CallFrame, SecretKey, @@ -56,7 +52,7 @@ pub fn generate_fee_collection_contract(address: Address) -> Vec { let asm = vec![ // Pointer to AssetID memory address in call frame param a - op::addi(0x10, RegId::FP, CallFrame::a_offset() as u16), + op::addi(0x10, RegId::FP, CallFrame::a_offset().try_into().unwrap()), // pointer to the withdrawal address embedded after the contract bytecode op::addi( 0x11, From 21ef2a19cb36939f9243a206714edd27edb64184 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 22 Nov 2023 19:04:46 +0200 Subject: [PATCH 04/14] Add test cases --- Cargo.lock | 3 + bin/fuel-core/Cargo.toml | 2 +- bin/fuel-core/src/cli/fee_contract.rs | 4 +- crates/chain-config/Cargo.toml | 3 + .../src/fee_collection_contract.rs | 295 ++++++++++++++++++ crates/chain-config/src/lib.rs | 54 +--- 6 files changed, 306 insertions(+), 55 deletions(-) create mode 100644 crates/chain-config/src/fee_collection_contract.rs diff --git a/Cargo.lock b/Cargo.lock index 4eeb0708ebf..84fe4d89433 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2855,6 +2855,8 @@ version = "0.21.0-rc.1" dependencies = [ "anyhow", "bech32", + "fuel-core", + "fuel-core-client", "fuel-core-storage", "fuel-core-types", "hex", @@ -2865,6 +2867,7 @@ dependencies = [ "serde", "serde_json", "serde_with", + "tokio", "tracing", ] diff --git a/bin/fuel-core/Cargo.toml b/bin/fuel-core/Cargo.toml index aeb579fdfa4..08ecf0e76c9 100644 --- a/bin/fuel-core/Cargo.toml +++ b/bin/fuel-core/Cargo.toml @@ -22,8 +22,8 @@ const_format = { version = "0.2", optional = true } dirs = "4.0" dotenvy = { version = "0.15", optional = true } fuel-core = { workspace = true } -fuel-core-types = { workspace = true } fuel-core-chain-config = { workspace = true } +fuel-core-types = { workspace = true } hex = "0.4" humantime = "2.1" lazy_static = { workspace = true } diff --git a/bin/fuel-core/src/cli/fee_contract.rs b/bin/fuel-core/src/cli/fee_contract.rs index ccf21492d67..741abcb4e50 100644 --- a/bin/fuel-core/src/cli/fee_contract.rs +++ b/bin/fuel-core/src/cli/fee_contract.rs @@ -1,5 +1,5 @@ use clap::Parser; -use fuel_core_chain_config::generate_fee_collection_contract; +use fuel_core_chain_config::fee_collection_contract; use fuel_core_types::fuel_tx::Address; use std::{ fs::OpenOptions, @@ -20,7 +20,7 @@ pub struct Command { } pub async fn exec(cmd: Command) -> anyhow::Result<()> { - let contract = generate_fee_collection_contract(cmd.withdrawal_address); + let contract = fee_collection_contract::generate(cmd.withdrawal_address); if let Some(output) = cmd.output.as_ref() { let mut open_opt = OpenOptions::new(); diff --git a/crates/chain-config/Cargo.toml b/crates/chain-config/Cargo.toml index 2f1ba39147c..05f99311a3d 100644 --- a/crates/chain-config/Cargo.toml +++ b/crates/chain-config/Cargo.toml @@ -25,10 +25,13 @@ serde_with = "1.11" tracing = "0.1" [dev-dependencies] +fuel-core = { workspace = true } +fuel-core-client = { workspace = true } fuel-core-types = { workspace = true, default-features = false, features = ["random", "serde"] } insta = { workspace = true } rand = { workspace = true } serde_json = { version = "1.0", features = ["raw_value"] } +tokio = { workspace = true, features = ["full"] } [features] default = ["std"] diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs new file mode 100644 index 00000000000..8317462715e --- /dev/null +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -0,0 +1,295 @@ +use fuel_core_types::{ + fuel_asm::{ + op, + Instruction, + RegId, + }, + fuel_tx::Address, + fuel_vm::CallFrame, +}; + +pub fn generate(address: Address) -> Vec { + let start_jump = vec![ + // Jump over the embedded address, which is placed immediately after the jump + op::ji((1 + (Address::LEN / Instruction::SIZE)).try_into().unwrap()), + ]; + + let body = vec![ + // Load pointer to AssetId memory address in call frame param a + op::addi(0x10, RegId::FP, CallFrame::a_offset().try_into().unwrap()), + op::lw(0x10, 0x10, 0), + // Get the balance of asset ID in the contract + op::bal(0x11, 0x10, RegId::FP), + // If balance == 0, return early + op::jnzf(0x11, RegId::ZERO, 1), + op::ret(RegId::ONE), + // Pointer to the embedded address + op::addi(0x12, RegId::IS, Instruction::SIZE.try_into().unwrap()), + // Perform the transfer + op::tro(0x12, RegId::ONE, 0x11, 0x10), + // Return + op::ret(RegId::ONE), + ]; + + let mut asm_bytes: Vec = start_jump.into_iter().collect(); + asm_bytes.extend_from_slice(address.as_slice()); // Embed the address + let body: Vec = body.into_iter().collect(); + asm_bytes.extend(body.as_slice()); + + asm_bytes +} + +#[cfg(test)] +mod tests { + // TODO: Tests to write for this contract: + // 1. Happy path withdrawal case for block producer + // Deploy the contract for the block producer's address + // Run some blocks and accumulate fees + // Attempt to withdraw the collected fees to the BP's address + // (note that currently we allow anyone to initiate this withdrawal) + // 2. Unhappy case where tx doesn't have the expected variable output set + // 3. Edge case, withdrawal is attempted when there are no fees collected (shouldn't revert, but the BP's balance should be the same) + + use super::*; + use crate::SecretKey; + + use rand::{ + rngs::StdRng, + Rng, + SeedableRng, + }; + + use fuel_core::service::{ + Config, + FuelService, + }; + use fuel_core_client::client::{ + types::TransactionStatus, + FuelClient, + }; + use fuel_core_types::{ + fuel_tx::{ + Cacheable, + Finalizable, + Input, + Output, + TransactionBuilder, + TxParameters, + Witness, + }, + fuel_types::{ + AssetId, + BlockHeight, + ChainId, + Salt, + }, + fuel_vm::{ + consts::VM_MAX_RAM, + script_with_data_offset, + }, + }; + + #[tokio::test] + async fn happy_path() { + let rng = &mut StdRng::seed_from_u64(0); + + // Make contract that coinbase fees are collected into + let address: Address = rng.gen(); + let salt: Salt = rng.gen(); + let contract = generate(address); + let witness: Witness = contract.into(); + let mut create_tx = TransactionBuilder::create(witness.clone(), salt, vec![]) + .add_random_fee_input() + .finalize(); + create_tx + .precompute(&ChainId::default()) + .expect("tx should be valid"); + let contract_id = create_tx.metadata().as_ref().unwrap().contract_id; + + // Start up a node + let mut config = Config::local_node(); + config.debug = true; + config.block_producer.coinbase_recipient = Some(contract_id); + let node = FuelService::new_node(config).await.unwrap(); + let client = FuelClient::from(node.bound_address); + + // Submit contract creation tx + let tx_status = client + .submit_and_await_commit(&create_tx.into()) + .await + .unwrap(); + assert!(matches!(tx_status, TransactionStatus::Success { .. })); + let bh = client.produce_blocks(1, None).await.unwrap(); + assert_eq!(bh, BlockHeight::new(2)); + + // No fees should have been collected yet + let contract_balance = + client.contract_balance(&(contract_id), None).await.unwrap(); + assert_eq!(contract_balance, 0); + + for i in 0..10 { + // Run a script that does nothing, but will cause fee collection + let tx = TransactionBuilder::script( + [op::ret(RegId::ONE)].into_iter().collect(), + vec![], + ) + .add_unsigned_coin_input( + SecretKey::random(rng), + rng.gen(), + 1000, + Default::default(), + Default::default(), + Default::default(), + ) + .gas_price(1) + .script_gas_limit(1_000_000) + .finalize_as_transaction(); + let tx_status = client.submit_and_await_commit(&tx).await.unwrap(); + assert!(matches!(tx_status, TransactionStatus::Success { .. })); + + // Now the coinbase fee should be reflected in the contract balance + let contract_balance = + client.contract_balance(&(contract_id), None).await.unwrap(); + assert_eq!(contract_balance, i + 1); + } + + // Now call the fee collection contract to withdraw the fees + let (script, _) = script_with_data_offset!( + data_offset, + vec![ + op::movi(0x10, AssetId::LEN.try_into().unwrap()), + op::aloc(0x10), + op::movi(0x10, data_offset), + op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), + op::ret(RegId::ONE), + ], + TxParameters::DEFAULT.tx_offset() + ); + let tx = TransactionBuilder::script( + script.into_iter().collect(), + (*contract_id) + .into_iter() + .chain((VM_MAX_RAM - (AssetId::LEN as u64)).to_be_bytes()) + .chain(0u64.to_be_bytes()) + .collect(), + ) + .add_random_fee_input() // No coinbase fee for this block + .gas_price(0) + .script_gas_limit(1_000_000) + .add_input(Input::contract( + Default::default(), + Default::default(), + Default::default(), + Default::default(), + contract_id, + )) + .add_output(Output::contract(1, Default::default(), Default::default())) + .add_output(Output::variable( + Default::default(), + Default::default(), + Default::default(), + )) + .finalize_as_transaction(); + + let tx_status = client.submit_and_await_commit(&tx).await.unwrap(); + assert!(matches!(tx_status, TransactionStatus::Success { .. })); + + // Make sure that the full balance was been withdrawn + let contract_balance = + client.contract_balance(&(contract_id), None).await.unwrap(); + assert_eq!(contract_balance, 0); + + // Make sure that the full balance was been withdrawn + let asset_balance = client.balance(&address, None).await.unwrap(); + assert_eq!(asset_balance, 10); + } + + #[tokio::test] + async fn no_fees_collected_yet() { + let rng = &mut StdRng::seed_from_u64(0); + + // Make contract that coinbase fees are collected into + let address: Address = rng.gen(); + let salt: Salt = rng.gen(); + let contract = generate(address); + let witness: Witness = contract.into(); + let mut create_tx = TransactionBuilder::create(witness.clone(), salt, vec![]) + .add_random_fee_input() + .finalize(); + create_tx + .precompute(&ChainId::default()) + .expect("tx should be valid"); + let contract_id = create_tx.metadata().as_ref().unwrap().contract_id; + + // Start up a node + let mut config = Config::local_node(); + config.debug = true; + config.block_producer.coinbase_recipient = Some(contract_id); + let node = FuelService::new_node(config).await.unwrap(); + let client = FuelClient::from(node.bound_address); + + // Submit contract creation tx + let tx_status = client + .submit_and_await_commit(&create_tx.into()) + .await + .unwrap(); + assert!(matches!(tx_status, TransactionStatus::Success { .. })); + let bh = client.produce_blocks(1, None).await.unwrap(); + assert_eq!(bh, BlockHeight::new(2)); + + // No fees should have been collected yet + let contract_balance = + client.contract_balance(&(contract_id), None).await.unwrap(); + assert_eq!(contract_balance, 0); + + // Now call the fee collection contract to withdraw the fees + let (script, _) = script_with_data_offset!( + data_offset, + vec![ + op::movi(0x10, AssetId::LEN.try_into().unwrap()), + op::aloc(0x10), + op::movi(0x10, data_offset), + op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), + op::ret(RegId::ONE), + ], + TxParameters::DEFAULT.tx_offset() + ); + let tx = TransactionBuilder::script( + script.into_iter().collect(), + (*contract_id) + .into_iter() + .chain((VM_MAX_RAM - (AssetId::LEN as u64)).to_be_bytes()) + .chain(0u64.to_be_bytes()) + .collect(), + ) + .add_random_fee_input() // No coinbase fee for this block + .gas_price(0) + .script_gas_limit(1_000_000) + .add_input(Input::contract( + Default::default(), + Default::default(), + Default::default(), + Default::default(), + contract_id, + )) + .add_output(Output::contract(1, Default::default(), Default::default())) + .add_output(Output::variable( + Default::default(), + Default::default(), + Default::default(), + )) + .finalize_as_transaction(); + + let tx_status = client.submit_and_await_commit(&tx).await.unwrap(); + assert!(matches!(tx_status, TransactionStatus::Success { .. })); + + // Make sure that the full balance was been withdrawn + let contract_balance = + client.contract_balance(&(contract_id), None).await.unwrap(); + assert_eq!(contract_balance, 0); + + // Make sure that the full balance was been withdrawn + let asset_balance = client.balance(&address, None).await.unwrap(); + assert_eq!(asset_balance, 0); + } +} diff --git a/crates/chain-config/src/lib.rs b/crates/chain-config/src/lib.rs index c472adf21ef..13d413dfc2d 100644 --- a/crates/chain-config/src/lib.rs +++ b/crates/chain-config/src/lib.rs @@ -4,21 +4,12 @@ #![deny(warnings)] pub mod config; +pub mod fee_collection_contract; mod genesis; mod serialization; pub use config::*; -use fuel_core_types::{ - fuel_asm::{ - op, - RegId, - }, - fuel_tx::Address, - fuel_vm::{ - CallFrame, - SecretKey, - }, -}; +use fuel_core_types::fuel_vm::SecretKey; pub use genesis::GenesisCommitment; /// A default secret key to use for testing purposes only @@ -35,44 +26,3 @@ pub fn default_consensus_dev_key() -> SecretKey { ]; SecretKey::try_from(bytes.as_slice()).expect("valid key") } - -pub fn generate_fee_collection_contract(address: Address) -> Vec { - // TODO: Tests to write for this contract: - // 1. Happy path withdrawal case for block producer - // Deploy the contract for the block producer's address - // Run some blocks and accumulate fees - // Attempt to withdraw the collected fees to the BP's address - // (note that currently we allow anyone to initiate this withdrawal) - // 2. Unhappy case where tx doesn't have the expected variable output set - // 3. Edge case, withdrawal is attempted when there are no fees collected (shouldn't revert, but the BP's balance should be the same) - - // TODO: setup cli interface for generating this contract - // This should be accessible to the fuel-core CLI. - // ie. something like `fuel-core generate-fee-contract ` -> - - let asm = vec![ - // Pointer to AssetID memory address in call frame param a - op::addi(0x10, RegId::FP, CallFrame::a_offset().try_into().unwrap()), - // pointer to the withdrawal address embedded after the contract bytecode - op::addi( - 0x11, - RegId::IS, - 7, // update when number of opcodes changes - ), - // get the balance of asset ID in the contract - op::bal(0x11, 0x10, RegId::FP), - // if balance > 0, withdraw - op::eq(0x12, 0x11, RegId::ZERO), - op::jnzf(0x12, RegId::ZERO, 1), - // todo: point $rA to the location of the withdrawal address in memory (after return) - op::tro(0, 0, 0x11, 0x10), - // return - op::ret(RegId::ONE), - ]; - - let mut asm_bytes: Vec = asm.into_iter().collect(); - // append withdrawal address at the end of the contract bytecode. - asm_bytes.extend_from_slice(address.as_slice()); - - asm_bytes -} From eccae9331d97c3aad2ca94711ad64f67116ff2bd Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 22 Nov 2023 19:16:20 +0200 Subject: [PATCH 05/14] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c51aec1ed33..6b4126bf4f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Description of the upcoming release here. - [#1263](https://github.com/FuelLabs/fuel-core/pull/1263): Add gas benchmarks for `ED19` and `ECR1` instructions. - [#1331](https://github.com/FuelLabs/fuel-core/pull/1331): Add peer reputation reporting to block import code - [#1405](https://github.com/FuelLabs/fuel-core/pull/1405): Use correct names for service metrics. +- [#1501](https://github.com/FuelLabs/fuel-core/pull/1501): Add a CLI command for generating a fee collection contract. ### Changed From f7da1e8d484cc989c6c6dd4328215d0bf330048b Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 22 Nov 2023 19:17:40 +0200 Subject: [PATCH 06/14] Refactor test to remove duplication --- .../src/fee_collection_contract.rs | 108 ++++++++++++------ 1 file changed, 76 insertions(+), 32 deletions(-) diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs index 8317462715e..3cdb92a92e8 100644 --- a/crates/chain-config/src/fee_collection_contract.rs +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -81,6 +81,7 @@ mod tests { AssetId, BlockHeight, ChainId, + ContractId, Salt, }, fuel_vm::{ @@ -89,10 +90,14 @@ mod tests { }, }; - #[tokio::test] - async fn happy_path() { - let rng = &mut StdRng::seed_from_u64(0); + struct TestContext { + address: Address, + contract_id: ContractId, + _node: FuelService, + client: FuelClient, + } + async fn setup(rng: &mut StdRng) -> TestContext { // Make contract that coinbase fees are collected into let address: Address = rng.gen(); let salt: Salt = rng.gen(); @@ -127,31 +132,21 @@ mod tests { client.contract_balance(&(contract_id), None).await.unwrap(); assert_eq!(contract_balance, 0); - for i in 0..10 { - // Run a script that does nothing, but will cause fee collection - let tx = TransactionBuilder::script( - [op::ret(RegId::ONE)].into_iter().collect(), - vec![], - ) - .add_unsigned_coin_input( - SecretKey::random(rng), - rng.gen(), - 1000, - Default::default(), - Default::default(), - Default::default(), - ) - .gas_price(1) - .script_gas_limit(1_000_000) - .finalize_as_transaction(); - let tx_status = client.submit_and_await_commit(&tx).await.unwrap(); - assert!(matches!(tx_status, TransactionStatus::Success { .. })); - - // Now the coinbase fee should be reflected in the contract balance - let contract_balance = - client.contract_balance(&(contract_id), None).await.unwrap(); - assert_eq!(contract_balance, i + 1); + TestContext { + address, + contract_id, + _node: node, + client, } + } + + async fn collect_fees(ctx: &TestContext) { + let TestContext { + client, + address, + contract_id, + .. + } = ctx; // Now call the fee collection contract to withdraw the fees let (script, _) = script_with_data_offset!( @@ -169,7 +164,7 @@ mod tests { script.into_iter().collect(), (*contract_id) .into_iter() - .chain((VM_MAX_RAM - (AssetId::LEN as u64)).to_be_bytes()) + .chain((VM_MAX_RAM.checked_sub(AssetId::LEN as u64)).unwrap().to_be_bytes()) .chain(0u64.to_be_bytes()) .collect(), ) @@ -181,7 +176,7 @@ mod tests { Default::default(), Default::default(), Default::default(), - contract_id, + *contract_id, )) .add_output(Output::contract(1, Default::default(), Default::default())) .add_output(Output::variable( @@ -195,12 +190,61 @@ mod tests { assert!(matches!(tx_status, TransactionStatus::Success { .. })); // Make sure that the full balance was been withdrawn - let contract_balance = - client.contract_balance(&(contract_id), None).await.unwrap(); + let contract_balance = client.contract_balance(contract_id, None).await.unwrap(); assert_eq!(contract_balance, 0); // Make sure that the full balance was been withdrawn - let asset_balance = client.balance(&address, None).await.unwrap(); + let asset_balance = client.balance(address, None).await.unwrap(); + assert_eq!(asset_balance, 10); + } + + #[tokio::test] + async fn happy_path() { + let rng = &mut StdRng::seed_from_u64(0); + + let ctx = setup(rng).await; + + for i in 0..10 { + // Run a script that does nothing, but will cause fee collection + let tx = TransactionBuilder::script( + [op::ret(RegId::ONE)].into_iter().collect(), + vec![], + ) + .add_unsigned_coin_input( + SecretKey::random(rng), + rng.gen(), + 1000, + Default::default(), + Default::default(), + Default::default(), + ) + .gas_price(1) + .script_gas_limit(1_000_000) + .finalize_as_transaction(); + let tx_status = ctx.client.submit_and_await_commit(&tx).await.unwrap(); + assert!(matches!(tx_status, TransactionStatus::Success { .. })); + + // Now the coinbase fee should be reflected in the contract balance + let contract_balance = ctx + .client + .contract_balance(&ctx.contract_id, None) + .await + .unwrap(); + assert_eq!(contract_balance, i + 1); + } + + collect_fees(&ctx).await; + + // Make sure that the full balance was been withdrawn + let contract_balance = ctx + .client + .contract_balance(&ctx.contract_id, None) + .await + .unwrap(); + assert_eq!(contract_balance, 0); + + // Make sure that the full balance was been withdrawn + let asset_balance = ctx.client.balance(&ctx.address, None).await.unwrap(); assert_eq!(asset_balance, 10); } From e4a5cf5e4a976ccb3954dd3fa41b2106f3be3fd1 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 22 Nov 2023 19:32:30 +0200 Subject: [PATCH 07/14] Add a test case with missing variable output --- .../src/fee_collection_contract.rs | 168 ++++++++---------- 1 file changed, 76 insertions(+), 92 deletions(-) diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs index 3cdb92a92e8..405eb9dd75e 100644 --- a/crates/chain-config/src/fee_collection_contract.rs +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -41,15 +41,6 @@ pub fn generate(address: Address) -> Vec { #[cfg(test)] mod tests { - // TODO: Tests to write for this contract: - // 1. Happy path withdrawal case for block producer - // Deploy the contract for the block producer's address - // Run some blocks and accumulate fees - // Attempt to withdraw the collected fees to the BP's address - // (note that currently we allow anyone to initiate this withdrawal) - // 2. Unhappy case where tx doesn't have the expected variable output set - // 3. Edge case, withdrawal is attempted when there are no fees collected (shouldn't revert, but the BP's balance should be the same) - use super::*; use crate::SecretKey; @@ -140,10 +131,46 @@ mod tests { } } + /// This makes a block with a single transaction that has a fee, + /// so that the coinbase fee is collected into the contract + async fn make_block_with_fee(rng: &mut StdRng, ctx: &TestContext) { + let old_balance = ctx + .client + .contract_balance(&ctx.contract_id, None) + .await + .unwrap(); + + // Run a script that does nothing, but will cause fee collection + let tx = TransactionBuilder::script( + [op::ret(RegId::ONE)].into_iter().collect(), + vec![], + ) + .add_unsigned_coin_input( + SecretKey::random(rng), + rng.gen(), + 1000, + Default::default(), + Default::default(), + Default::default(), + ) + .gas_price(1) + .script_gas_limit(1_000_000) + .finalize_as_transaction(); + let tx_status = ctx.client.submit_and_await_commit(&tx).await.unwrap(); + assert!(matches!(tx_status, TransactionStatus::Success { .. })); + + // Now the coinbase fee should be reflected in the contract balance + let new_balance = ctx + .client + .contract_balance(&ctx.contract_id, None) + .await + .unwrap(); + assert_eq!(new_balance, old_balance.checked_add(1).unwrap()); + } + async fn collect_fees(ctx: &TestContext) { let TestContext { client, - address, contract_id, .. } = ctx; @@ -188,14 +215,6 @@ mod tests { let tx_status = client.submit_and_await_commit(&tx).await.unwrap(); assert!(matches!(tx_status, TransactionStatus::Success { .. })); - - // Make sure that the full balance was been withdrawn - let contract_balance = client.contract_balance(contract_id, None).await.unwrap(); - assert_eq!(contract_balance, 0); - - // Make sure that the full balance was been withdrawn - let asset_balance = client.balance(address, None).await.unwrap(); - assert_eq!(asset_balance, 10); } #[tokio::test] @@ -204,33 +223,8 @@ mod tests { let ctx = setup(rng).await; - for i in 0..10 { - // Run a script that does nothing, but will cause fee collection - let tx = TransactionBuilder::script( - [op::ret(RegId::ONE)].into_iter().collect(), - vec![], - ) - .add_unsigned_coin_input( - SecretKey::random(rng), - rng.gen(), - 1000, - Default::default(), - Default::default(), - Default::default(), - ) - .gas_price(1) - .script_gas_limit(1_000_000) - .finalize_as_transaction(); - let tx_status = ctx.client.submit_and_await_commit(&tx).await.unwrap(); - assert!(matches!(tx_status, TransactionStatus::Success { .. })); - - // Now the coinbase fee should be reflected in the contract balance - let contract_balance = ctx - .client - .contract_balance(&ctx.contract_id, None) - .await - .unwrap(); - assert_eq!(contract_balance, i + 1); + for _ in 0..10 { + make_block_with_fee(rng, &ctx).await; } collect_fees(&ctx).await; @@ -248,45 +242,36 @@ mod tests { assert_eq!(asset_balance, 10); } + /// Attempts fee collection when no balance has accumulated yet #[tokio::test] async fn no_fees_collected_yet() { let rng = &mut StdRng::seed_from_u64(0); - // Make contract that coinbase fees are collected into - let address: Address = rng.gen(); - let salt: Salt = rng.gen(); - let contract = generate(address); - let witness: Witness = contract.into(); - let mut create_tx = TransactionBuilder::create(witness.clone(), salt, vec![]) - .add_random_fee_input() - .finalize(); - create_tx - .precompute(&ChainId::default()) - .expect("tx should be valid"); - let contract_id = create_tx.metadata().as_ref().unwrap().contract_id; - - // Start up a node - let mut config = Config::local_node(); - config.debug = true; - config.block_producer.coinbase_recipient = Some(contract_id); - let node = FuelService::new_node(config).await.unwrap(); - let client = FuelClient::from(node.bound_address); + let ctx = setup(rng).await; + collect_fees(&ctx).await; - // Submit contract creation tx - let tx_status = client - .submit_and_await_commit(&create_tx.into()) + // Make sure that the balance is still zero + let contract_balance = ctx + .client + .contract_balance(&ctx.contract_id, None) .await .unwrap(); - assert!(matches!(tx_status, TransactionStatus::Success { .. })); - let bh = client.produce_blocks(1, None).await.unwrap(); - assert_eq!(bh, BlockHeight::new(2)); - - // No fees should have been collected yet - let contract_balance = - client.contract_balance(&(contract_id), None).await.unwrap(); assert_eq!(contract_balance, 0); - // Now call the fee collection contract to withdraw the fees + // There were no coins to withdraw + let asset_balance = ctx.client.balance(&ctx.address, None).await.unwrap(); + assert_eq!(asset_balance, 0); + } + + #[tokio::test] + async fn missing_variable_output() { + let rng = &mut StdRng::seed_from_u64(0); + + let ctx = setup(rng).await; + make_block_with_fee(rng, &ctx).await; + + // Now call the fee collection contract to withdraw the fees, + // but unlike in the happy path, we don't add the variable output let (script, _) = script_with_data_offset!( data_offset, vec![ @@ -300,9 +285,9 @@ mod tests { ); let tx = TransactionBuilder::script( script.into_iter().collect(), - (*contract_id) + (*ctx.contract_id) .into_iter() - .chain((VM_MAX_RAM - (AssetId::LEN as u64)).to_be_bytes()) + .chain((VM_MAX_RAM.checked_sub(AssetId::LEN as u64)).unwrap().to_be_bytes()) .chain(0u64.to_be_bytes()) .collect(), ) @@ -314,26 +299,25 @@ mod tests { Default::default(), Default::default(), Default::default(), - contract_id, + ctx.contract_id, )) .add_output(Output::contract(1, Default::default(), Default::default())) - .add_output(Output::variable( - Default::default(), - Default::default(), - Default::default(), - )) .finalize_as_transaction(); - let tx_status = client.submit_and_await_commit(&tx).await.unwrap(); - assert!(matches!(tx_status, TransactionStatus::Success { .. })); + let tx_status = ctx.client.submit_and_await_commit(&tx).await.unwrap(); + let TransactionStatus::Failure { reason, .. } = tx_status else { + panic!("Expected failure"); + }; + assert_eq!(reason, "OutputNotFound"); - // Make sure that the full balance was been withdrawn - let contract_balance = - client.contract_balance(&(contract_id), None).await.unwrap(); - assert_eq!(contract_balance, 0); - - // Make sure that the full balance was been withdrawn - let asset_balance = client.balance(&address, None).await.unwrap(); + // Make sure that nothing was withdrawn + let contract_balance = ctx + .client + .contract_balance(&ctx.contract_id, None) + .await + .unwrap(); + assert_eq!(contract_balance, 1); + let asset_balance = ctx.client.balance(&ctx.address, None).await.unwrap(); assert_eq!(asset_balance, 0); } } From fe2d928c4e82b3d630d2c735d3dcb2e13ce9483e Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 22 Nov 2023 20:19:43 +0200 Subject: [PATCH 08/14] Make output index contract parameter, and fix issues from review --- crates/chain-config/src/fee_collection_contract.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs index 405eb9dd75e..85e1e7b10fd 100644 --- a/crates/chain-config/src/fee_collection_contract.rs +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -18,6 +18,9 @@ pub fn generate(address: Address) -> Vec { // Load pointer to AssetId memory address in call frame param a op::addi(0x10, RegId::FP, CallFrame::a_offset().try_into().unwrap()), op::lw(0x10, 0x10, 0), + // Load output index from call frame param b + op::addi(0x13, RegId::FP, CallFrame::b_offset().try_into().unwrap()), + op::lw(0x13, 0x13, 0), // Get the balance of asset ID in the contract op::bal(0x11, 0x10, RegId::FP), // If balance == 0, return early @@ -26,7 +29,7 @@ pub fn generate(address: Address) -> Vec { // Pointer to the embedded address op::addi(0x12, RegId::IS, Instruction::SIZE.try_into().unwrap()), // Perform the transfer - op::tro(0x12, RegId::ONE, 0x11, 0x10), + op::tro(0x12, 0x13, 0x11, 0x10), // Return op::ret(RegId::ONE), ]; @@ -165,7 +168,7 @@ mod tests { .contract_balance(&ctx.contract_id, None) .await .unwrap(); - assert_eq!(new_balance, old_balance.checked_add(1).unwrap()); + assert!(new_balance > old_balance); } async fn collect_fees(ctx: &TestContext) { @@ -192,7 +195,7 @@ mod tests { (*contract_id) .into_iter() .chain((VM_MAX_RAM.checked_sub(AssetId::LEN as u64)).unwrap().to_be_bytes()) - .chain(0u64.to_be_bytes()) + .chain(1u64.to_be_bytes()) // Output index .collect(), ) .add_random_fee_input() // No coinbase fee for this block @@ -214,6 +217,7 @@ mod tests { .finalize_as_transaction(); let tx_status = client.submit_and_await_commit(&tx).await.unwrap(); + dbg!(&tx_status); assert!(matches!(tx_status, TransactionStatus::Success { .. })); } @@ -288,7 +292,7 @@ mod tests { (*ctx.contract_id) .into_iter() .chain((VM_MAX_RAM.checked_sub(AssetId::LEN as u64)).unwrap().to_be_bytes()) - .chain(0u64.to_be_bytes()) + .chain(1u64.to_be_bytes()) // Output index .collect(), ) .add_random_fee_input() // No coinbase fee for this block From d30f09d3b7c8d8254c0935be99ff930a26fb7772 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 22 Nov 2023 20:31:57 +0200 Subject: [PATCH 09/14] Add docs about call structure --- crates/chain-config/src/fee_collection_contract.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs index 85e1e7b10fd..21e8b43d5e1 100644 --- a/crates/chain-config/src/fee_collection_contract.rs +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -182,8 +182,10 @@ mod tests { let (script, _) = script_with_data_offset!( data_offset, vec![ + // Allocate space for the AssetId op::movi(0x10, AssetId::LEN.try_into().unwrap()), op::aloc(0x10), + // Point to the call structure op::movi(0x10, data_offset), op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), op::ret(RegId::ONE), @@ -279,8 +281,10 @@ mod tests { let (script, _) = script_with_data_offset!( data_offset, vec![ + // Allocate space for the AssetId op::movi(0x10, AssetId::LEN.try_into().unwrap()), op::aloc(0x10), + // Point to the call structure op::movi(0x10, data_offset), op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), op::ret(RegId::ONE), From 417d91335d1c2d4167da4d4dedc214b5b33d4bd0 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 22 Nov 2023 20:32:33 +0200 Subject: [PATCH 10/14] Docs --- crates/chain-config/src/fee_collection_contract.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs index 21e8b43d5e1..08f041e99ae 100644 --- a/crates/chain-config/src/fee_collection_contract.rs +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -182,7 +182,7 @@ mod tests { let (script, _) = script_with_data_offset!( data_offset, vec![ - // Allocate space for the AssetId + // Allocate all-zeros AssetId op::movi(0x10, AssetId::LEN.try_into().unwrap()), op::aloc(0x10), // Point to the call structure @@ -281,7 +281,7 @@ mod tests { let (script, _) = script_with_data_offset!( data_offset, vec![ - // Allocate space for the AssetId + // Allocate all-zeros AssetId op::movi(0x10, AssetId::LEN.try_into().unwrap()), op::aloc(0x10), // Point to the call structure From 699341304ac2c2f6641fd5f0892e92a9f55246d7 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 22 Nov 2023 20:33:08 +0200 Subject: [PATCH 11/14] More docs --- crates/chain-config/src/fee_collection_contract.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs index 08f041e99ae..3bd36028815 100644 --- a/crates/chain-config/src/fee_collection_contract.rs +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -8,6 +8,10 @@ use fuel_core_types::{ fuel_vm::CallFrame, }; +/// Generates the bytecode for the fee collection contract. +/// The contract takes two parameters: +/// - a: pointer to the AssetId memory address +/// - b: output index pub fn generate(address: Address) -> Vec { let start_jump = vec![ // Jump over the embedded address, which is placed immediately after the jump From eb2ecba0ee0ce3d55019113ccaa31261c461cd87 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 22 Nov 2023 20:49:14 +0200 Subject: [PATCH 12/14] Use GTF for script data location --- .../src/fee_collection_contract.rs | 51 ++++++++----------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs index 3bd36028815..d7dda9c2fc9 100644 --- a/crates/chain-config/src/fee_collection_contract.rs +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -66,13 +66,13 @@ mod tests { FuelClient, }; use fuel_core_types::{ + fuel_asm::GTFArgs, fuel_tx::{ Cacheable, Finalizable, Input, Output, TransactionBuilder, - TxParameters, Witness, }, fuel_types::{ @@ -82,10 +82,7 @@ mod tests { ContractId, Salt, }, - fuel_vm::{ - consts::VM_MAX_RAM, - script_with_data_offset, - }, + fuel_vm::consts::VM_MAX_RAM, }; struct TestContext { @@ -183,19 +180,15 @@ mod tests { } = ctx; // Now call the fee collection contract to withdraw the fees - let (script, _) = script_with_data_offset!( - data_offset, - vec![ - // Allocate all-zeros AssetId - op::movi(0x10, AssetId::LEN.try_into().unwrap()), - op::aloc(0x10), - // Point to the call structure - op::movi(0x10, data_offset), - op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), - op::ret(RegId::ONE), - ], - TxParameters::DEFAULT.tx_offset() - ); + let script = vec![ + // Allocate all-zeros AssetId + op::movi(0x10, AssetId::LEN.try_into().unwrap()), + op::aloc(0x10), + // Point to the call structure + op::gtf_args(0x10, 0x00, GTFArgs::ScriptData), + op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), + op::ret(RegId::ONE), + ]; let tx = TransactionBuilder::script( script.into_iter().collect(), (*contract_id) @@ -282,19 +275,15 @@ mod tests { // Now call the fee collection contract to withdraw the fees, // but unlike in the happy path, we don't add the variable output - let (script, _) = script_with_data_offset!( - data_offset, - vec![ - // Allocate all-zeros AssetId - op::movi(0x10, AssetId::LEN.try_into().unwrap()), - op::aloc(0x10), - // Point to the call structure - op::movi(0x10, data_offset), - op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), - op::ret(RegId::ONE), - ], - TxParameters::DEFAULT.tx_offset() - ); + let script = vec![ + // Allocate all-zeros AssetId + op::movi(0x10, AssetId::LEN.try_into().unwrap()), + op::aloc(0x10), + // Point to the call structure + op::gtf_args(0x10, 0x00, GTFArgs::ScriptData), + op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), + op::ret(RegId::ONE), + ]; let tx = TransactionBuilder::script( script.into_iter().collect(), (*ctx.contract_id) From ed06e89f095d54fe3cfaa9753f4a9dc9bf2c48ef Mon Sep 17 00:00:00 2001 From: xgreenx Date: Wed, 22 Nov 2023 21:41:18 +0000 Subject: [PATCH 13/14] Fetch `AssetId` and `output_index` from `script_data` --- .../src/fee_collection_contract.rs | 155 +++++++++++++----- 1 file changed, 110 insertions(+), 45 deletions(-) diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs index d7dda9c2fc9..eb552671fc1 100644 --- a/crates/chain-config/src/fee_collection_contract.rs +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -1,39 +1,59 @@ use fuel_core_types::{ fuel_asm::{ op, + GTFArgs, Instruction, RegId, }, - fuel_tx::Address, - fuel_vm::CallFrame, + fuel_tx::{ + Address, + AssetId, + }, }; /// Generates the bytecode for the fee collection contract. -/// The contract takes two parameters: -/// - a: pointer to the AssetId memory address -/// - b: output index +/// The contract expects `AssetId` and `output_index` as a first elements in `script_data`. pub fn generate(address: Address) -> Vec { let start_jump = vec![ // Jump over the embedded address, which is placed immediately after the jump op::ji((1 + (Address::LEN / Instruction::SIZE)).try_into().unwrap()), ]; + let asset_id_register = 0x10; + let balance_register = 0x11; + let contract_id_register = 0x12; + let output_index_register = 0x13; + let recipient_id_register = 0x14; let body = vec![ - // Load pointer to AssetId memory address in call frame param a - op::addi(0x10, RegId::FP, CallFrame::a_offset().try_into().unwrap()), - op::lw(0x10, 0x10, 0), - // Load output index from call frame param b - op::addi(0x13, RegId::FP, CallFrame::b_offset().try_into().unwrap()), - op::lw(0x13, 0x13, 0), + // Load pointer to AssetId + op::gtf_args(asset_id_register, 0x00, GTFArgs::ScriptData), + // Load output index + op::addi( + output_index_register, + asset_id_register, + AssetId::LEN as u16, + ), + op::lw(output_index_register, output_index_register, 0), + // Gets pointer to the contract id + op::move_(contract_id_register, RegId::FP), // Get the balance of asset ID in the contract - op::bal(0x11, 0x10, RegId::FP), + op::bal(balance_register, asset_id_register, contract_id_register), // If balance == 0, return early - op::jnzf(0x11, RegId::ZERO, 1), + op::jnzf(balance_register, RegId::ZERO, 1), op::ret(RegId::ONE), - // Pointer to the embedded address - op::addi(0x12, RegId::IS, Instruction::SIZE.try_into().unwrap()), + // Pointer to the recipient address + op::addi( + recipient_id_register, + RegId::IS, + Instruction::SIZE.try_into().unwrap(), + ), // Perform the transfer - op::tro(0x12, 0x13, 0x11, 0x10), + op::tro( + recipient_id_register, + output_index_register, + balance_register, + asset_id_register, + ), // Return op::ret(RegId::ONE), ]; @@ -76,13 +96,13 @@ mod tests { Witness, }, fuel_types::{ + canonical::Serialize, AssetId, BlockHeight, ChainId, ContractId, Salt, }, - fuel_vm::consts::VM_MAX_RAM, }; struct TestContext { @@ -179,22 +199,29 @@ mod tests { .. } = ctx; + let asset_id = AssetId::BASE; + let output_index = 1u64; + let call_struct_register = 0x10; // Now call the fee collection contract to withdraw the fees let script = vec![ - // Allocate all-zeros AssetId - op::movi(0x10, AssetId::LEN.try_into().unwrap()), - op::aloc(0x10), // Point to the call structure - op::gtf_args(0x10, 0x00, GTFArgs::ScriptData), - op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), + op::gtf_args(call_struct_register, 0x00, GTFArgs::ScriptData), + op::addi( + call_struct_register, + call_struct_register, + (asset_id.size() + output_index.size()) as u16, + ), + op::call(call_struct_register, RegId::ZERO, RegId::ZERO, RegId::CGAS), op::ret(RegId::ONE), ]; + let tx = TransactionBuilder::script( - script.into_iter().collect(), - (*contract_id) - .into_iter() - .chain((VM_MAX_RAM.checked_sub(AssetId::LEN as u64)).unwrap().to_be_bytes()) - .chain(1u64.to_be_bytes()) // Output index + script.into_iter().collect(),asset_id.to_bytes().into_iter() + .chain(output_index.to_bytes().into_iter()) + .chain(contract_id + .to_bytes().into_iter()) + .chain(0u64.to_bytes().into_iter()) + .chain(0u64.to_bytes().into_iter()) .collect(), ) .add_random_fee_input() // No coinbase fee for this block @@ -216,8 +243,10 @@ mod tests { .finalize_as_transaction(); let tx_status = client.submit_and_await_commit(&tx).await.unwrap(); - dbg!(&tx_status); - assert!(matches!(tx_status, TransactionStatus::Success { .. })); + assert!( + matches!(tx_status, TransactionStatus::Success { .. }), + "{tx_status:?}" + ); } #[tokio::test] @@ -230,19 +259,35 @@ mod tests { make_block_with_fee(rng, &ctx).await; } + // When + // Before withdrawal, the recipient's balance should be zero, + // and the contract balance should be non-zero. + let contract_balance_before_collect = ctx + .client + .contract_balance(&ctx.contract_id, None) + .await + .unwrap(); + assert_ne!(contract_balance_before_collect, 0); + assert_eq!(ctx.client.balance(&ctx.address, None).await.unwrap(), 0); + + // When collect_fees(&ctx).await; + // Then + // Make sure that the full balance was been withdrawn - let contract_balance = ctx + let contract_balance_after_collect = ctx .client .contract_balance(&ctx.contract_id, None) .await .unwrap(); - assert_eq!(contract_balance, 0); + assert_eq!(contract_balance_after_collect, 0); // Make sure that the full balance was been withdrawn - let asset_balance = ctx.client.balance(&ctx.address, None).await.unwrap(); - assert_eq!(asset_balance, 10); + assert_eq!( + ctx.client.balance(&ctx.address, None).await.unwrap(), + contract_balance_before_collect + ); } /// Attempts fee collection when no balance has accumulated yet @@ -251,8 +296,21 @@ mod tests { let rng = &mut StdRng::seed_from_u64(0); let ctx = setup(rng).await; + + // Given + let contract_balance_before_collect = ctx + .client + .contract_balance(&ctx.contract_id, None) + .await + .unwrap(); + assert_eq!(contract_balance_before_collect, 0); + assert_eq!(ctx.client.balance(&ctx.address, None).await.unwrap(), 0); + + // When collect_fees(&ctx).await; + // Then + // Make sure that the balance is still zero let contract_balance = ctx .client @@ -262,8 +320,7 @@ mod tests { assert_eq!(contract_balance, 0); // There were no coins to withdraw - let asset_balance = ctx.client.balance(&ctx.address, None).await.unwrap(); - assert_eq!(asset_balance, 0); + assert_eq!(ctx.client.balance(&ctx.address, None).await.unwrap(), 0); } #[tokio::test] @@ -273,23 +330,31 @@ mod tests { let ctx = setup(rng).await; make_block_with_fee(rng, &ctx).await; + let asset_id = AssetId::BASE; + let output_index = 1u64; + let call_struct_register = 0x10; + // Now call the fee collection contract to withdraw the fees, - // but unlike in the happy path, we don't add the variable output + // but unlike in the happy path, we don't add the variable output to the transaction. let script = vec![ - // Allocate all-zeros AssetId - op::movi(0x10, AssetId::LEN.try_into().unwrap()), - op::aloc(0x10), // Point to the call structure - op::gtf_args(0x10, 0x00, GTFArgs::ScriptData), - op::call(0x10, RegId::ZERO, RegId::ZERO, RegId::CGAS), + op::gtf_args(call_struct_register, 0x00, GTFArgs::ScriptData), + op::addi( + call_struct_register, + call_struct_register, + (asset_id.size() + output_index.size()) as u16, + ), + op::call(call_struct_register, RegId::ZERO, RegId::ZERO, RegId::CGAS), op::ret(RegId::ONE), ]; let tx = TransactionBuilder::script( script.into_iter().collect(), - (*ctx.contract_id) - .into_iter() - .chain((VM_MAX_RAM.checked_sub(AssetId::LEN as u64)).unwrap().to_be_bytes()) - .chain(1u64.to_be_bytes()) // Output index + asset_id.to_bytes().into_iter() + .chain(output_index.to_bytes().into_iter()) + .chain(ctx.contract_id + .to_bytes().into_iter()) + .chain(0u64.to_bytes().into_iter()) + .chain(0u64.to_bytes().into_iter()) .collect(), ) .add_random_fee_input() // No coinbase fee for this block From 6882b1e61981b8856b163b0e54d13c706a94eed7 Mon Sep 17 00:00:00 2001 From: xgreenx Date: Wed, 22 Nov 2023 21:49:09 +0000 Subject: [PATCH 14/14] Make clippy happy --- crates/chain-config/src/fee_collection_contract.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/chain-config/src/fee_collection_contract.rs b/crates/chain-config/src/fee_collection_contract.rs index eb552671fc1..780ebe16174 100644 --- a/crates/chain-config/src/fee_collection_contract.rs +++ b/crates/chain-config/src/fee_collection_contract.rs @@ -31,7 +31,7 @@ pub fn generate(address: Address) -> Vec { op::addi( output_index_register, asset_id_register, - AssetId::LEN as u16, + u16::try_from(AssetId::LEN).expect("The size is 32"), ), op::lw(output_index_register, output_index_register, 0), // Gets pointer to the contract id @@ -67,6 +67,8 @@ pub fn generate(address: Address) -> Vec { } #[cfg(test)] +#[allow(clippy::cast_possible_truncation)] +#[allow(clippy::arithmetic_side_effects)] mod tests { use super::*; use crate::SecretKey;