Skip to content

Commit

Permalink
fix: ensure policies are respected (#1423)
Browse files Browse the repository at this point in the history
Ensures that set gas limit and max fee policies take precedence over
estimated values.

### Checklist
- [ ] I have linked to any relevant issues.
- [ ] I have updated the documentation.
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added necessary labels.
- [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [ ] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: hal3e <git@hal3e.io>
  • Loading branch information
MujkicA and hal3e authored Jun 13, 2024
1 parent 3c37548 commit 960cfc2
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 20 deletions.
59 changes: 59 additions & 0 deletions e2e/tests/providers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,3 +1008,62 @@ async fn can_upload_executor_and_trigger_upgrade() -> Result<()> {

Ok(())
}

#[tokio::test]
async fn tx_respects_policies() -> Result<()> {
setup_program_test!(
Wallets("wallet"),
Abigen(Contract(
name = "TestContract",
project = "e2e/sway/contracts/contract_test"
)),
Deploy(
name = "contract_instance",
contract = "TestContract",
wallet = "wallet"
),
);

let tip = 22;
let witness_limit = 1000;
let maturity = 4;
let max_fee = 10_000;
let script_gas_limit = 3000;
let tx_policies = TxPolicies::new(
Some(tip),
Some(witness_limit),
Some(maturity),
Some(max_fee),
Some(script_gas_limit),
);

// advance the block height to ensure the maturity is respected
let provider = wallet.try_provider()?;
provider.produce_blocks(4, None).await?;

// trigger a transaction that contains script code to verify
// that policies precede estimated values
let response = contract_instance
.methods()
.initialize_counter(42)
.with_tx_policies(tx_policies)
.call()
.await?;

let tx_response = provider
.get_transaction_by_id(&response.tx_id.unwrap())
.await?
.expect("tx should exist");
let script = match tx_response.transaction {
TransactionType::Script(tx) => tx,
_ => panic!("expected script transaction"),
};

assert_eq!(script.maturity(), maturity as u32);
assert_eq!(script.tip().unwrap(), tip);
assert_eq!(script.witness_limit().unwrap(), witness_limit);
assert_eq!(script.max_fee().unwrap(), max_fee);
assert_eq!(script.gas_limit(), script_gas_limit);

Ok(())
}
5 changes: 2 additions & 3 deletions e2e/tests/wallets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ async fn send_transfer_transactions() -> Result<()> {
// Configure transaction policies
let tip = 2;
let script_gas_limit = 500_000;
let expected_script_gas_limit = 0;
let maturity = 0;

let tx_policies = TxPolicies::default()
Expand All @@ -280,8 +279,8 @@ async fn send_transfer_transactions() -> Result<()> {
TransactionType::Script(tx) => tx,
_ => panic!("Received unexpected tx type!"),
};
// Transfer scripts have `script_gas_limit` set to `0`
assert_eq!(script.gas_limit(), expected_script_gas_limit);
// Transfer scripts uses set `script_gas_limit` despite not having script code
assert_eq!(script.gas_limit(), script_gas_limit);
assert_eq!(script.maturity(), maturity as u32);

let wallet_1_spendable_resources = wallet_1.get_spendable_resources(base_asset_id, 1).await?;
Expand Down
46 changes: 31 additions & 15 deletions packages/fuels-core/src/types/transaction_builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,12 +509,16 @@ impl ScriptTransactionBuilder {
self.set_script_gas_limit(&mut script_dry_runner, &mut tx)
.await?;

Self::set_max_fee_policy(
&mut tx,
&dry_runner,
self.gas_price_estimation_block_horizon,
)
.await?;
if let Some(max_fee) = self.tx_policies.max_fee() {
tx.policies_mut().set(PolicyType::MaxFee, Some(max_fee));
} else {
Self::set_max_fee_policy(
&mut tx,
&dry_runner,
self.gas_price_estimation_block_horizon,
)
.await?;
}

self.set_witnesses(&mut tx, dry_runner).await?;

Expand All @@ -537,11 +541,11 @@ impl ScriptTransactionBuilder {
tx: &mut fuel_tx::Script,
) -> Result<()> {
let has_no_code = self.script.is_empty();
let script_gas_limit = if has_no_code {
0
} else if let Some(gas_limit) = self.tx_policies.script_gas_limit() {
let script_gas_limit = if let Some(gas_limit) = self.tx_policies.script_gas_limit() {
// Use the user defined value even if it makes the transaction revert.
gas_limit
} else if has_no_code {
0
} else {
let dry_run = if let Some(dry_run) = dry_runner.last_dry_run() {
// Even if the last dry run included variable outputs they only affect the transaction fee,
Expand Down Expand Up @@ -749,8 +753,12 @@ impl CreateTransactionBuilder {
self.witnesses,
);

Self::set_max_fee_policy(&mut tx, provider, self.gas_price_estimation_block_horizon)
.await?;
if let Some(max_fee) = self.tx_policies.max_fee() {
tx.policies_mut().set(PolicyType::MaxFee, Some(max_fee));
} else {
Self::set_max_fee_policy(&mut tx, &provider, self.gas_price_estimation_block_horizon)
.await?;
}

let missing_witnesses =
generate_missing_witnesses(tx.id(&chain_id), &self.unresolved_signers).await?;
Expand Down Expand Up @@ -847,8 +855,12 @@ impl UploadTransactionBuilder {
self.witnesses,
);

Self::set_max_fee_policy(&mut tx, provider, self.gas_price_estimation_block_horizon)
.await?;
if let Some(max_fee) = self.tx_policies.max_fee() {
tx.policies_mut().set(PolicyType::MaxFee, Some(max_fee));
} else {
Self::set_max_fee_policy(&mut tx, &provider, self.gas_price_estimation_block_horizon)
.await?;
}

let missing_witnesses =
generate_missing_witnesses(tx.id(&chain_id), &self.unresolved_signers).await?;
Expand Down Expand Up @@ -947,8 +959,12 @@ impl UpgradeTransactionBuilder {
self.witnesses,
);

Self::set_max_fee_policy(&mut tx, provider, self.gas_price_estimation_block_horizon)
.await?;
if let Some(max_fee) = self.tx_policies.max_fee() {
tx.policies_mut().set(PolicyType::MaxFee, Some(max_fee));
} else {
Self::set_max_fee_policy(&mut tx, &provider, self.gas_price_estimation_block_horizon)
.await?;
}

let missing_witnesses =
generate_missing_witnesses(tx.id(&chain_id), &self.unresolved_signers).await?;
Expand Down
23 changes: 21 additions & 2 deletions packages/fuels-core/src/types/wrappers/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ use async_trait::async_trait;
use fuel_crypto::{Message, Signature};
use fuel_tx::{
field::{
Inputs, Maturity, MintAmount, MintAssetId, Outputs, Script as ScriptField, ScriptData,
ScriptGasLimit, WitnessLimit, Witnesses,
Inputs, Maturity, MintAmount, MintAssetId, Outputs, Policies as PoliciesField,
Script as ScriptField, ScriptData, ScriptGasLimit, WitnessLimit, Witnesses,
},
input::{
coin::{CoinPredicate, CoinSigned},
message::{
MessageCoinPredicate, MessageCoinSigned, MessageDataPredicate, MessageDataSigned,
},
},
policies::PolicyType,
Bytes32, Cacheable, Chargeable, ConsensusParameters, Create, FormatValidityChecks, Input, Mint,
Output, Salt as FuelSalt, Script, StorageSlot, Transaction as FuelTransaction, TransactionFee,
UniqueIdentifier, Upgrade, Upload, Witness,
Expand Down Expand Up @@ -250,6 +251,12 @@ pub trait Transaction:

fn witnesses(&self) -> &Vec<Witness>;

fn max_fee(&self) -> Option<u64>;

fn witness_limit(&self) -> Option<u64>;

fn tip(&self) -> Option<u64>;

fn is_using_predicates(&self) -> bool;

/// Precompute transaction metadata. The metadata is required for
Expand Down Expand Up @@ -445,6 +452,18 @@ macro_rules! impl_tx_wrapper {
Ok(self.tx.precompute(chain_id)?)
}

fn max_fee(&self) -> Option<u64> {
self.tx.policies().get(PolicyType::MaxFee)
}

fn witness_limit(&self) -> Option<u64> {
self.tx.policies().get(PolicyType::WitnessLimit)
}

fn tip(&self) -> Option<u64> {
self.tx.policies().get(PolicyType::Tip)
}

fn append_witness(&mut self, witness: Witness) -> Result<usize> {
let witness_size = calculate_witnesses_size(
self.tx.witnesses().iter().chain(std::iter::once(&witness)),
Expand Down

0 comments on commit 960cfc2

Please sign in to comment.