Skip to content

Commit

Permalink
feat(wallet): default unversioned inputs + fix cucumber (#1198)
Browse files Browse the repository at this point in the history
Description
---
feat(wallet): default unversioned inputs + fix cucmber

Motivation and Context
---
Concurrency cucumber failing due to use of versioned inputs. Needed a
way to tell the wallet not to use versioned inputs when autodetecting
them

How Has This Been Tested?
---
Cucumber

What process can a PR reviewer use to test or verify this change?
---
Submit a transaction to the wallet and set
`detect_inputs_use_unversioned = true`

Breaking Changes
---

- [x] None
- [ ] Requires data directory to be deleted
- [ ] Other - Please specify
  • Loading branch information
sdbondi authored Nov 12, 2024
1 parent 9907bc1 commit b878f58
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 25 deletions.
2 changes: 2 additions & 0 deletions applications/tari_dan_wallet_cli/src/command/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ pub async fn handle_submit(args: SubmitArgs, client: &mut WalletDaemonClient) ->
signing_key_index: None,
autofill_inputs: vec![],
detect_inputs: common.detect_inputs.unwrap_or(true),
detect_inputs_use_unversioned: true,
proof_ids: vec![],
};
let resp = client.submit_transaction(&request).await?;
Expand Down Expand Up @@ -347,6 +348,7 @@ async fn handle_submit_manifest(
signing_key_index: None,
autofill_inputs: vec![],
detect_inputs: common.detect_inputs.unwrap_or(true),
detect_inputs_use_unversioned: true,
proof_ids: vec![],
};

Expand Down
19 changes: 16 additions & 3 deletions applications/tari_dan_wallet_daemon/src/handlers/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pub async fn handle_submit_instruction(
signing_key_index: Some(fee_account.key_index),
autofill_inputs: vec![],
detect_inputs: req.override_inputs.unwrap_or_default(),
detect_inputs_use_unversioned: false,
proof_ids: vec![],
};
handle_submit(context, token, request).await
Expand Down Expand Up @@ -106,15 +107,23 @@ pub async fn handle_submit(
loaded_substates
.into_iter()
.chain(substates.into_iter().map(SubstateRequirement::unversioned))
.map(|mut input| {
if req.detect_inputs_use_unversioned {
input.version = None;
}
input
})
.collect()
} else {
vec![]
};

info!(
target: LOG_TARGET,
"Detected {} input(s)",
detected_inputs.len()
"Detected {} input(s) (detect_inputs = {}, detect_inputs_use_unversioned = {})",
detected_inputs.len(),
req.detect_inputs,
req.detect_inputs_use_unversioned,
);

let transaction = Transaction::builder()
Expand All @@ -123,6 +132,10 @@ pub async fn handle_submit(
.sign(&key.key)
.build();

for input in transaction.inputs() {
debug!(target: LOG_TARGET, "Input: {}", input)
}

for proof_id in req.proof_ids {
// update the proofs table with the corresponding transaction hash
sdk.confidential_outputs_api()
Expand All @@ -137,7 +150,7 @@ pub async fn handle_submit(

let transaction_id = context
.transaction_service()
.submit_transaction(transaction, autofill_inputs.clone())
.submit_transaction(transaction, autofill_inputs)
.await?;

Ok(TransactionSubmitResponse { transaction_id })
Expand Down
11 changes: 11 additions & 0 deletions clients/wallet_daemon_client/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,22 @@ pub struct TransactionSubmitRequest {
#[cfg_attr(feature = "ts", ts(type = "number | null"))]
pub signing_key_index: Option<u64>,
pub autofill_inputs: Vec<SubstateRequirement>,
/// Attempt to infer inputs and their dependencies from instructions. If false, the provided transaction must
/// contain the required inputs.
pub detect_inputs: bool,
/// If true(default), detected inputs will omit versions allowing consensus to resolve input substates.
/// If false, the wallet will try determine versioned for the inputs. These may be outdated if the substate has
/// changed since detection.
#[serde(default = "return_true")]
pub detect_inputs_use_unversioned: bool,
#[cfg_attr(feature = "ts", ts(type = "Array<number>"))]
pub proof_ids: Vec<ConfidentialProofId>,
}

const fn return_true() -> bool {
true
}

#[derive(Debug, Clone, Deserialize, Serialize)]
#[cfg_attr(
feature = "ts",
Expand Down
39 changes: 32 additions & 7 deletions integration_tests/src/wallet_daemon_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ pub async fn transfer_confidential(
signing_key_index: Some(signing_key_index),
proof_ids: vec![proof_id],
detect_inputs: true,
detect_inputs_use_unversioned: false,
autofill_inputs: vec![source_account_addr, dest_account_addr],
};

Expand Down Expand Up @@ -475,6 +476,7 @@ pub async fn submit_manifest_with_signing_keys(
transaction,
signing_key_index: Some(account.key_index),
detect_inputs: true,
detect_inputs_use_unversioned: false,
proof_ids: vec![],
autofill_inputs: inputs,
};
Expand Down Expand Up @@ -555,6 +557,7 @@ pub async fn submit_manifest(
transaction,
signing_key_index: Some(account.key_index),
detect_inputs: true,
detect_inputs_use_unversioned: false,
proof_ids: vec![],
autofill_inputs: inputs,
};
Expand Down Expand Up @@ -604,6 +607,7 @@ pub async fn submit_transaction(
transaction,
signing_key_index: None,
detect_inputs: true,
detect_inputs_use_unversioned: false,
autofill_inputs: inputs,
proof_ids: vec![],
};
Expand Down Expand Up @@ -661,6 +665,7 @@ pub async fn create_component(
transaction,
signing_key_index: Some(account.key_index),
detect_inputs: true,
detect_inputs_use_unversioned: false,
proof_ids: vec![],
autofill_inputs: vec![],
};
Expand Down Expand Up @@ -716,6 +721,7 @@ pub async fn call_component(
wallet_daemon_name: String,
function_call: String,
new_outputs_name: Option<String>,
use_unversioned_inputs: bool,
) -> anyhow::Result<TransactionWaitResultResponse> {
let mut client = get_auth_wallet_daemon_client(world, &wallet_daemon_name).await;

Expand All @@ -734,10 +740,14 @@ pub async fn call_component(
.as_component_address()
.expect("Failed to get account component address");

let tx = Transaction::builder()
.fee_transaction_pay_from_component(account_component_address, Amount(1000))
.call_method(source_component_address, &function_call, vec![])
.with_inputs(vec![
let inputs = if use_unversioned_inputs {
[
SubstateRequirement::unversioned(account_component_address),
SubstateRequirement::unversioned(source_component_address),
]
} else {
// Typically only used in failing tests to assert that a substate is already DOWN
[
SubstateRequirement::new(
account_component_address.into(),
find_output_version(world, output_ref.as_str(), account_component_address.into())?,
Expand All @@ -746,10 +756,16 @@ pub async fn call_component(
source_component_address.into(),
find_output_version(world, output_ref.as_str(), source_component_address.into())?,
),
])
]
};

let tx = Transaction::builder()
.fee_transaction_pay_from_component(account_component_address, Amount(1000))
.call_method(source_component_address, &function_call, vec![])
.with_inputs(inputs)
.build_unsigned_transaction();

let resp = submit_unsigned_tx_and_wait_for_response(client, tx, account).await?;
let resp = submit_unsigned_tx_and_wait_for_response(client, tx, account, use_unversioned_inputs).await?;

let final_outputs_name = if let Some(name) = new_outputs_name {
name
Expand Down Expand Up @@ -779,6 +795,10 @@ pub async fn concurrent_call_component(
function_call: String,
times: usize,
) -> anyhow::Result<()> {
log::info!(
"concurrent_call_component: account_name={account_name}, output_ref={output_ref}, \
wallet_daemon_name={wallet_daemon_name}"
);
let mut client = get_auth_wallet_daemon_client(world, &wallet_daemon_name).await;

let source_component_address = get_address_from_output(world, output_ref.clone())
Expand All @@ -799,7 +819,7 @@ pub async fn concurrent_call_component(
.fee_transaction_pay_from_component(account_component_address, Amount(1000))
.call_method(source_component_address, &function_call, vec![])
.build_unsigned_transaction();
join_set.spawn(submit_unsigned_tx_and_wait_for_response(clt, tx, acc));
join_set.spawn(submit_unsigned_tx_and_wait_for_response(clt, tx, acc, true));
}

while let Some(result) = join_set.join_next().await {
Expand Down Expand Up @@ -911,12 +931,17 @@ async fn submit_unsigned_tx_and_wait_for_response(
mut client: WalletDaemonClient,
transaction: UnsignedTransaction,
account: Account,
use_unversioned_inputs: bool,
) -> anyhow::Result<TransactionWaitResultResponse> {
log::info!(
"submit_unsigned_tx_and_wait_for_response: account={account}, use_unversioned_inputs={use_unversioned_inputs}",
);
let submit_req = TransactionSubmitRequest {
transaction,
signing_key_index: Some(account.key_index),
autofill_inputs: vec![],
detect_inputs: true,
detect_inputs_use_unversioned: use_unversioned_inputs,
proof_ids: vec![],
};

Expand Down
29 changes: 24 additions & 5 deletions integration_tests/tests/cucumber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,16 @@ async fn call_wallet_daemon_method_and_check_result(
method_call: String,
expected_result: String,
) -> anyhow::Result<()> {
let resp =
wallet_daemon_cli::call_component(world, account_name, output_ref, wallet_daemon_name, method_call, None)
.await?;
let resp = wallet_daemon_cli::call_component(
world,
account_name,
output_ref,
wallet_daemon_name,
method_call,
None,
true,
)
.await?;

let finalize_result = resp
.result
Expand Down Expand Up @@ -416,7 +423,16 @@ async fn call_wallet_daemon_method(
output_ref: String,
method_call: String,
) -> anyhow::Result<()> {
wallet_daemon_cli::call_component(world, account_name, output_ref, wallet_daemon_name, method_call, None).await?;
wallet_daemon_cli::call_component(
world,
account_name,
output_ref,
wallet_daemon_name,
method_call,
None,
true,
)
.await?;

Ok(())
}
Expand All @@ -439,6 +455,7 @@ async fn call_wallet_daemon_method_with_output_name(
wallet_daemon_name,
method_call,
Some(new_output_name),
true,
)
.await?;

Expand All @@ -464,6 +481,8 @@ async fn call_wallet_daemon_method_with_output_name_error_result(
wallet_daemon_name,
method_call,
Some(new_output_name),
// We expect this to fail due to a substate being downed so we need to use versioned inputs
false,
)
.await
{
Expand All @@ -477,7 +496,7 @@ async fn call_wallet_daemon_method_with_output_name_error_result(
);
}
} else {
bail!("Error expected, but none was happening!");
bail!("Error expected, but the transaction succeeded.");
}

Ok(())
Expand Down
5 changes: 2 additions & 3 deletions integration_tests/tests/features/concurrency.feature
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
@concurrency
Feature: Concurrency

@ignore
Scenario: Concurrent calls to the Counter template

##### Setup
Expand Down Expand Up @@ -43,8 +42,8 @@ Feature: Concurrency
# Send multiple concurrent transactions to increase the counter
# Currently there is a lock bug where the subsequent transactions executed are being rejected, should be tested later after engine changes:
# Reject(FailedToLockInputs("Failed to Write lock substate component_459d...4443c:1 due to conflict with existing Write lock"))
When I invoke on wallet daemon WALLET_D on account ACC on component COUNTER/components/Counter the method call "increase" concurrently 2 times
When I invoke on wallet daemon WALLET_D on account ACC on component COUNTER/components/Counter the method call "increase" concurrently 30 times

# Check that the counter has been increased
# Note: this is currently not working together with the previous test case when times > 1, only the first transaction is being executed properly
When I invoke on wallet daemon WALLET_D on account ACC on component COUNTER/components/Counter the method call "value" the result is "2"
When I invoke on wallet daemon WALLET_D on account ACC on component COUNTER/components/Counter the method call "value" the result is "30"
3 changes: 2 additions & 1 deletion integration_tests/tests/features/state_sync.feature
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
@state_sync
Feature: State Sync

@flaky
# Ignore: this sometimes fails on CI but passes locally
@ignore
Scenario: New validator node registers and syncs
# Initialize a base node, wallet, miner and VN
Given a base node BASE
Expand Down
18 changes: 12 additions & 6 deletions integration_tests/tests/steps/validator_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,9 @@ async fn when_i_wait_for_validator_leaf_block_at_least(world: &mut TariWorld, na

if let Some(block) = resp.blocks.first() {
assert!(block.epoch().as_u64() <= epoch);
if block.epoch().as_u64() < epoch {
eprintln!("VN {name} is in {}. Waiting for epoch {epoch}", block.epoch())
}
if block.epoch().as_u64() == epoch && block.height().as_u64() >= height {
return;
}
Expand All @@ -468,16 +471,19 @@ async fn when_i_wait_for_validator_leaf_block_at_least(world: &mut TariWorld, na
})
.await
.unwrap();
let actual_height = resp
let block = resp
.blocks
.first()
.unwrap_or_else(|| panic!("Validator {name} has no blocks"))
.height()
.as_u64();
if actual_height < height {
.unwrap_or_else(|| panic!("Validator {name} has no blocks"));
if block.epoch().as_u64() < epoch {
panic!("Validator {} at {} is less than epoch {}", name, block.epoch(), epoch);
}
if block.height().as_u64() < height {
panic!(
"Validator {} leaf block height {} is less than {}",
name, actual_height, height
name,
block.height().as_u64(),
height
);
}
}
Expand Down

0 comments on commit b878f58

Please sign in to comment.