Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contract input set estimation #628

Merged
merged 34 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a66c56f
test receipt
Salka1988 Oct 13, 2022
1ef189c
add set_contracts_automatic
Salka1988 Oct 15, 2022
1599c60
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Oct 18, 2022
ba7d747
merge set_contracts_automatic and estimate_tx_dependencies
Salka1988 Oct 19, 2022
37d0368
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Oct 19, 2022
5e69140
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Oct 24, 2022
13ae5b5
add multicall test for id estimation
Salka1988 Oct 24, 2022
8999690
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Oct 26, 2022
741d22c
bump fuel-core
Salka1988 Oct 26, 2022
5631594
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 2, 2022
3718a8a
add fix
Salka1988 Nov 2, 2022
0b5ed00
add fix
Salka1988 Nov 2, 2022
82e13f0
add fmt
Salka1988 Nov 2, 2022
3320e5b
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 4, 2022
3e624f2
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 5, 2022
51b65b2
update fuel-core
Salka1988 Nov 5, 2022
8cb94fe
Merge branch 'Salka1988/contract_set_estimation' of github.com:FuelLa…
Salka1988 Nov 5, 2022
2e2e96e
fix tests
Salka1988 Nov 6, 2022
eb98122
change CI
Salka1988 Nov 6, 2022
9e0260c
add fmt
Salka1988 Nov 6, 2022
8d84551
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 6, 2022
347cfa4
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 9, 2022
8929017
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 9, 2022
49a5778
change FUEL_CORE_VERSION in CI
Salka1988 Nov 10, 2022
75f7a06
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 10, 2022
210d312
add call that does not need contract id
Salka1988 Nov 10, 2022
ddebf7a
Merge remote-tracking branch 'origin/Salka1988/contract_set_estimatio…
Salka1988 Nov 10, 2022
cb25b27
add fix
Salka1988 Nov 10, 2022
53777c8
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 10, 2022
47b9b21
add fix
Salka1988 Nov 10, 2022
ae51748
add fixes
Salka1988 Nov 11, 2022
249857b
add error message
Salka1988 Nov 11, 2022
c72039b
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 11, 2022
8a236ac
Merge branch 'master' into Salka1988/contract_set_estimation
Salka1988 Nov 14, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ env:
RUSTFLAGS: "-D warnings"
FUEL_CORE_VERSION: 0.14.1
RUST_VERSION: 1.64.0
FORC_VERSION: 0.30.0
FORC_VERSION: 0.30.1

jobs:
setup-test-projects:
Expand Down
56 changes: 55 additions & 1 deletion packages/fuels-contract/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::marker::PhantomData;
use std::path::Path;
use std::str::FromStr;

use fuel_gql_client::prelude::PanicReason;
use fuel_gql_client::{
fuel_tx::{Contract as FuelContract, Output, Receipt, StorageSlot, Transaction},
fuel_types::{Address, AssetId, Salt},
Expand Down Expand Up @@ -378,6 +379,10 @@ impl ContractCall {
}
}

pub fn append_external_contracts(&mut self, contract_id: Bech32ContractId) {
self.external_contracts.push(contract_id)
}

pub fn append_message_outputs(&mut self, num: u64) {
let new_message_outputs = vec![
Output::Message {
Expand Down Expand Up @@ -438,6 +443,12 @@ impl ContractCall {
let decoded_value = ABIDecoder::decode_single(&self.output_param, &encoded_value)?;
Ok(decoded_value)
}

fn find_contract_not_in_inputs(receipts: &[Receipt]) -> Option<&Receipt> {
receipts.iter().find(
|r| matches!(r, Receipt::Panic { reason, .. } if *reason.reason() == PanicReason::ContractNotInInputs ),
)
}
}

#[derive(Debug)]
Expand Down Expand Up @@ -465,6 +476,16 @@ where
self
}

/// Appends additional external contracts as dependencies to this contract's call.
/// Effectively, this will be used to create additional Input::Contract/Output::Contract
/// pairs and set them into the transaction.
/// Note that this is a builder method, i.e. use it as a chain:
/// `my_contract_instance.my_method(...).append_contracts(additional_contract_id).call()`.
pub fn append_contract(mut self, contract_id: Bech32ContractId) -> Self {
self.contract_call.append_external_contracts(contract_id);
self
}

/// Sets the transaction parameters for a given transaction.
/// Note that this is a builder method, i.e. use it as a chain:
/// let params = TxParameters { gas_price: 100, gas_limit: 1000000 };
Expand Down Expand Up @@ -541,6 +562,16 @@ where
Self::call_or_simulate(&self, true).await
}

/// Simulates a call without needing to resolve the generic for the return type
async fn simulate_without_decode(&self) -> Result<(), Error> {
let script = self.get_call_execution_script().await?;
let provider = self.wallet.get_provider()?;

script.simulate(provider).await?;

Ok(())
}

/// Simulates the call and attempts to resolve missing tx dependencies.
/// Forwards the received error if it cannot be fixed.
pub async fn estimate_tx_dependencies(
Expand All @@ -550,14 +581,24 @@ where
let attempts = max_attempts.unwrap_or(DEFAULT_TX_DEP_ESTIMATION_ATTEMPTS);

for _ in 0..attempts {
let result = self.call_or_simulate(true).await;
let result = self.simulate_without_decode().await;

match result {
Err(Error::RevertTransactionError(_, receipts))
if ContractCall::is_missing_output_variables(&receipts) =>
{
self = self.append_variable_outputs(1);
}

Err(Error::RevertTransactionError(_, ref receipts)) => {
if let Some(receipt) = ContractCall::find_contract_not_in_inputs(receipts) {
let contract_id = Bech32ContractId::from(*receipt.contract_id().unwrap());
self = self.append_contract(contract_id);
} else {
return Err(result.err().unwrap());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error here will be very cryptic when returned to the user.

The user will be trying to estimate_tx_dependencies, but if it is an Error::RevertTransactionError and for some weird reason that's out of our control, ContractCall::find_contract_not_in_inputs returns None, the Err here will be very hard to understand.

So we should return this Err with a string saying something like "Couldn't estimate tx dependencies because we couldn't find the missing contract input".

Copy link
Contributor

@MujkicA MujkicA Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If ContractCall::find_contract_not_in_inputs returns None it should mean that the Error::RevertTransactionError was due to reasons other than missing output variables or contract inputs. We should forward the original error to the user in that case since it cannot be fixed by estimating dependencies anyways. ~~
I need to improve my reading skills

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the initial idea.

}
}

Err(e) => return Err(e),
_ => return Ok(self),
}
Expand Down Expand Up @@ -694,6 +735,19 @@ impl MultiContractCallHandler {
.take(1)
.for_each(|call| call.append_variable_outputs(1));
}

Err(Error::RevertTransactionError(_, ref receipts)) => {
if let Some(receipt) = ContractCall::find_contract_not_in_inputs(receipts) {
let contract_id = Bech32ContractId::from(*receipt.contract_id().unwrap());
self.contract_calls
.iter_mut()
.take(1)
Salka1988 marked this conversation as resolved.
Show resolved Hide resolved
.for_each(|call| call.append_external_contracts(contract_id.clone()));
} else {
return Err(result.err().unwrap());
Salka1988 marked this conversation as resolved.
Show resolved Hide resolved
}
}

Err(e) => return Err(e),
_ => return Ok(self),
}
Expand Down
3 changes: 2 additions & 1 deletion packages/fuels-contract/src/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use fuel_gql_client::fuel_tx::{
field::Script as ScriptField, ConsensusParameters, Input, Output, Receipt, Transaction,
TxPointer, UtxoId,
};

use fuel_gql_client::fuel_types::{
bytes::padded_len_usize, AssetId, Bytes32, ContractId, Immediate18, Word,
};
Expand Down Expand Up @@ -410,7 +411,7 @@ impl Script {
.iter()
.any(|r|
matches!(r, Receipt::ScriptResult { result, .. } if *result != ScriptExecutionResult::Success)
) {
) {
return Err(Error::RevertTransactionError(Default::default(), receipts));
}

Expand Down
92 changes: 92 additions & 0 deletions packages/fuels/tests/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -645,3 +645,95 @@ async fn contract_call_futures_implement_send() -> Result<(), Error> {

Ok(())
}

#[tokio::test]
async fn test_contract_set_estimation() -> Result<(), Error> {
setup_contract_test!(
foo_contract_instance,
wallet,
"packages/fuels/tests/contracts/foo_contract"
);
let foo_contract_id = foo_contract_instance.get_contract_id();

let res = foo_contract_instance.methods().foo(true).call().await?;
assert!(!res.value);

setup_contract_test!(
foo_caller_contract_instance,
None,
"packages/fuels/tests/contracts/foo_caller_contract"
);

let bits = *foo_contract_id.hash();

{
// Should fail due to missing external contracts
let res = foo_caller_contract_instance
.methods()
.call_foo_contract(Bits256(bits), true)
.call()
.await;
assert!(matches!(res, Err(Error::RevertTransactionError(..))));
}

let res = foo_caller_contract_instance
.methods()
.call_foo_contract(Bits256(bits), true)
.estimate_tx_dependencies(None)
Salka1988 marked this conversation as resolved.
Show resolved Hide resolved
.await?
.call()
.await?;

assert!(res.value);
Ok(())
}

#[tokio::test]
async fn test_output_variable_contract_id_estimation_multicall() -> Result<(), Error> {
setup_contract_test!(
foo_contract_instance,
wallet,
"packages/fuels/tests/contracts/foo_contract"
);

let foo_contract_id = foo_contract_instance.get_contract_id();

setup_contract_test!(
foo_caller_contract_instance,
None,
"packages/fuels/tests/contracts/foo_caller_contract"
);

setup_contract_test!(
contract_test_instance,
None,
"packages/fuels/tests/contracts/contract_test"
);

let bits = *foo_contract_id.hash();
let contract_methods = foo_caller_contract_instance.methods();

let mut multi_call_handler = MultiContractCallHandler::new(wallet.clone());
multi_call_handler.tx_params(Default::default());

(0..3).for_each(|_| {
let call_handler = contract_methods.call_foo_contract(Bits256(bits), true);
multi_call_handler.add_call(call_handler);
});
Salka1988 marked this conversation as resolved.
Show resolved Hide resolved

// add call that does not need ContractId
let contract_methods = contract_test_instance.methods();
let call_handler = contract_methods.get(5, 6);

multi_call_handler.add_call(call_handler);

let call_response = multi_call_handler
.estimate_tx_dependencies(None)
.await?
.call::<(bool, bool, bool, u64)>()
.await?;

assert_eq!(call_response.value, (true, true, true, 5));

Ok(())
}