Skip to content

Commit

Permalink
fix: minor improvements to template lib and test tooling (#1038)
Browse files Browse the repository at this point in the history
Description
---
minor ergonomic improvements to test tooling
add missing serde feature to validator_node_client crate
better access denied message for component access rules
better error message for failed `extract_component_value` (previously
unwrap on None)
deprecate `create_owned_account` in favour of `create_funded_account`
add `assert_contains_no_confidential_funds` method to Bucket
return_value accessor for test transaction  results

Motivation and Context
---
Minor clarity and ergonomics changes for template testing.
The missing serde "rc" feature caused compile to fail for
`transaction_submitter`

How Has This Been Tested?
---
Existing tests

What process can a PR reviewer use to test or verify this change?
---
CI

Breaking Changes
---

- [x] None
- [ ] Requires data directory to be deleted
- [ ] Other - Please specify
  • Loading branch information
sdbondi authored May 21, 2024
1 parent cbcd92f commit 4b8382f
Show file tree
Hide file tree
Showing 20 changed files with 109 additions and 56 deletions.
2 changes: 1 addition & 1 deletion clients/validator_node_client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ tari_dan_storage = { workspace = true }

reqwest = { workspace = true, features = ["json"] }
multiaddr = { workspace = true }
serde = { workspace = true, default-features = true }
serde = { workspace = true, default-features = true, features = ["rc"] }
serde_json = { workspace = true }
thiserror = { workspace = true }
ts-rs = { workspace = true, optional = true }
Expand Down
13 changes: 10 additions & 3 deletions dan_layer/engine/src/runtime/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@ use std::fmt::{Display, Formatter};
use tari_template_lib::{
args::{ComponentAction, VaultAction},
auth::ResourceAuthAction,
models::ComponentAddress,
};

#[derive(Debug, Clone)]
pub enum ActionIdent {
Native(NativeAction),
ComponentCallMethod { method: String },
ComponentCallMethod {
component_address: ComponentAddress,
method: String,
},
}

impl From<NativeAction> for ActionIdent {
Expand Down Expand Up @@ -42,8 +46,11 @@ impl Display for ActionIdent {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
ActionIdent::Native(native_fn) => write!(f, "native.{}", native_fn),
ActionIdent::ComponentCallMethod { method } => {
write!(f, "call component method '{}'", method)
ActionIdent::ComponentCallMethod {
component_address,
method,
} => {
write!(f, "call component method '{method}' on {component_address}")
},
}
}
Expand Down
11 changes: 11 additions & 0 deletions dan_layer/engine/src/runtime/tracker_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,29 @@ impl<'a> Authorization<'a> {
return Ok(());
}

let component_address =
locked
.address()
.as_component_address()
.ok_or_else(|| RuntimeError::InvariantError {
function: "check_component_access_rules",
details: format!("Expected a component address, got {}", locked.address()),
})?;

// Check access rules
match component.access_rules().get_method_access_rule(method) {
AccessRule::AllowAll => Ok(()),
AccessRule::DenyAll => Err(RuntimeError::AccessDenied {
action_ident: ActionIdent::ComponentCallMethod {
component_address,
method: method.to_string(),
},
}),
AccessRule::Restricted(rule) => {
if !check_restricted_access_rule(self.state, scope, rule)? {
return Err(RuntimeError::AccessDenied {
action_ident: ActionIdent::ComponentCallMethod {
component_address,
method: method.to_string(),
},
});
Expand Down
2 changes: 2 additions & 0 deletions dan_layer/engine/tests/access_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ mod component_access_rules {
);

assert_access_denied_for_action(reason, ActionIdent::ComponentCallMethod {
component_address,
method: "set_value".to_string(),
});
}
Expand Down Expand Up @@ -137,6 +138,7 @@ mod component_access_rules {
);

assert_access_denied_for_action(reason, ActionIdent::ComponentCallMethod {
component_address,
method: "set_value".to_string(),
});

Expand Down
11 changes: 6 additions & 5 deletions dan_layer/engine/tests/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ fn basic_faucet_transfer() {
.unwrap();

// Create sender and receiver accounts
let (sender_address, sender_proof, _) = template_test.create_owned_account();
let (receiver_address, _, _) = template_test.create_owned_account();
let (sender_address, sender_proof, _) = template_test.create_funded_account();
let (receiver_address, _, _) = template_test.create_funded_account();

let _result = template_test
.execute_and_commit(
Expand Down Expand Up @@ -128,7 +128,7 @@ fn withdraw_from_account_prevented() {
.unwrap();

// Create sender and receiver accounts
let (source_account, _, _) = template_test.create_owned_account();
let (source_account, _, _) = template_test.create_funded_account();

let _result = template_test
.execute_and_commit_manifest(
Expand All @@ -147,7 +147,7 @@ fn withdraw_from_account_prevented() {
)
.unwrap();

let (dest_address, non_owning_token, non_owning_key) = template_test.create_owned_account();
let (dest_address, non_owning_token, non_owning_key) = template_test.create_funded_account();

let reason = template_test.execute_expect_failure(
Transaction::builder()
Expand All @@ -161,6 +161,7 @@ fn withdraw_from_account_prevented() {
);

assert_access_denied_for_action(reason, ActionIdent::ComponentCallMethod {
component_address: source_account,
method: "withdraw".to_string(),
});

Expand Down Expand Up @@ -190,7 +191,7 @@ fn attempt_to_overwrite_account() {
let mut template_test = TemplateTest::new::<_, &str>([]);

// Create initial account with faucet funds
let (source_account, source_account_proof, source_account_sk) = template_test.create_owned_account();
let (source_account, source_account_proof, source_account_sk) = template_test.create_funded_account();
let source_account_pk = RistrettoPublicKey::from_secret_key(&source_account_sk);

let overwriting_tx = template_test.execute_expect_failure(
Expand Down
2 changes: 1 addition & 1 deletion dan_layer/engine/tests/account_nfts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn basic_nft_mint() {

let account_nft_template = account_nft_template_test.get_template_address("AccountNonFungible");

let (owner_component_address, owner_token, _) = account_nft_template_test.create_owned_account();
let (owner_component_address, owner_token, _) = account_nft_template_test.create_funded_account();

let result = account_nft_template_test
.execute_and_commit(
Expand Down
5 changes: 3 additions & 2 deletions dan_layer/engine/tests/composability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ fn it_allows_method_to_function_calls() {
fn it_fails_on_invalid_calls() {
let mut test = setup();
let components = initialize_composability(&mut test);
let (_, _, private_key) = test.template_test.create_owned_account();
let (_, _, private_key) = test.template_test.create_funded_account();

// the "invalid_state_call" method tries to call a non-existent method in the inner state component
let result = test
Expand Down Expand Up @@ -298,6 +298,7 @@ fn it_does_not_propagate_permissions() {

// We expect the component call to withdraw to fail with AccessDenied.
assert_access_denied_for_action(reason, ActionIdent::ComponentCallMethod {
component_address: victim_account,
method: "withdraw".to_string(),
});
}
Expand Down Expand Up @@ -331,7 +332,7 @@ fn it_allows_multiple_recursion_levels() {
#[test]
fn it_fails_when_surpassing_recursion_limit() {
let mut test = setup();
let (_, _, private_key) = test.template_test.create_owned_account();
let (_, _, private_key) = test.template_test.create_funded_account();
let max_call_depth = MAX_CALL_DEPTH;

// innermost composability component
Expand Down
16 changes: 8 additions & 8 deletions dan_layer/engine/tests/confidential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ fn transfer_confidential_amounts_between_accounts() {
let (mut template_test, faucet, faucet_resx) = setup(confidential_proof, None);

// Create an account
let (account1, owner1, _k) = template_test.create_owned_account();
let (account2, _owner2, _k) = template_test.create_owned_account();
let (account1, owner1, _k) = template_test.create_funded_account();
let (account2, _owner2, _k) = template_test.create_funded_account();

// Create proof for transfer
let proof = generate_withdraw_proof(&faucet_mask, Amount(1000), Some(Amount(99_000)), Amount(0));
Expand Down Expand Up @@ -179,7 +179,7 @@ fn transfer_confidential_fails_with_invalid_balance() {
let (mut template_test, faucet, _faucet_resx) = setup(confidential_proof, None);

// Create an account
let (account1, _owner1, _k) = template_test.create_owned_account();
let (account1, _owner1, _k) = template_test.create_funded_account();

// Create proof for transfer
let proof = generate_withdraw_proof(&faucet_mask, Amount(1001), Some(Amount(99_000)), Amount(0));
Expand Down Expand Up @@ -211,8 +211,8 @@ fn reveal_confidential_and_transfer() {
let (mut template_test, faucet, faucet_resx) = setup(confidential_proof, None);

// Create an account
let (account1, owner1, _k) = template_test.create_owned_account();
let (account2, owner2, _k) = template_test.create_owned_account();
let (account1, owner1, _k) = template_test.create_funded_account();
let (account2, owner2, _k) = template_test.create_funded_account();

// Create proof for transfer

Expand Down Expand Up @@ -284,8 +284,8 @@ fn attempt_to_reveal_with_unbalanced_proof() {
let (mut template_test, faucet, faucet_resx) = setup(confidential_proof, None);

// Create an account
let (account1, owner1, _k) = template_test.create_owned_account();
let (account2, _owner2, _k) = template_test.create_owned_account();
let (account1, owner1, _k) = template_test.create_funded_account();
let (account2, _owner2, _k) = template_test.create_funded_account();

// Create proof for transfer

Expand Down Expand Up @@ -337,7 +337,7 @@ fn multi_commitment_join() {
let (mut template_test, faucet, faucet_resx) = setup(confidential_proof, None);

// Create an account
let (account1, owner1, _k) = template_test.create_owned_account();
let (account1, owner1, _k) = template_test.create_funded_account();

// Create proof for transfer

Expand Down
6 changes: 3 additions & 3 deletions dan_layer/engine/tests/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn basic_emit_event() {
fn cannot_use_standard_topic() {
let mut template_test = TemplateTest::new(vec!["tests/templates/events"]);
let event_emitter_template = template_test.get_template_address("EventEmitter");
let (_, _, private_key) = template_test.create_owned_account();
let (_, _, private_key) = template_test.create_funded_account();
let invalid_topic = "std.mytopic";
let reason = template_test.execute_expect_failure(
Transaction::builder()
Expand Down Expand Up @@ -82,8 +82,8 @@ fn builtin_vault_events() {
.unwrap();

// Create sender and receiver accounts
let (sender_address, sender_proof, _) = template_test.create_owned_account();
let (receiver_address, _, _) = template_test.create_owned_account();
let (sender_address, sender_proof, _) = template_test.create_funded_account();
let (receiver_address, _, _) = template_test.create_funded_account();
template_test
.execute_and_commit(
vec![
Expand Down
24 changes: 12 additions & 12 deletions dan_layer/engine/tests/fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use tari_transaction::Transaction;
fn deducts_fees_from_payments_and_refunds_the_rest() {
let mut test = TemplateTest::new(["tests/templates/state"]);

let (account, owner_token, private_key) = test.create_owned_account();
let (account, owner_token, private_key) = test.create_funded_account();
let orig_balance: Amount = test.call_method(account, "balance", args![CONFIDENTIAL_TARI_RESOURCE_ADDRESS], vec![]);

test.enable_fees();
Expand Down Expand Up @@ -44,7 +44,7 @@ fn deducts_fees_from_payments_and_refunds_the_rest() {
fn deducts_fees_when_transaction_fails() {
let mut test = TemplateTest::new(["tests/templates/state"]);

let (account, owner_token, private_key) = test.create_owned_account();
let (account, owner_token, private_key) = test.create_funded_account();
let orig_balance: Amount = test.call_method(account, "balance", args![CONFIDENTIAL_TARI_RESOURCE_ADDRESS], vec![]);

test.enable_fees();
Expand Down Expand Up @@ -120,7 +120,7 @@ fn another_account_pays_partially_for_fees() {
let mut test = TemplateTest::new(iter::empty::<&str>());

let (account, owner_token, private_key) = test.create_empty_account();
let (account_fee, owner_token_fee, _) = test.create_owned_account();
let (account_fee, owner_token_fee, _) = test.create_funded_account();
let orig_balance: Amount = test.call_method(
account_fee,
"balance",
Expand Down Expand Up @@ -167,7 +167,7 @@ fn another_account_pays_partially_for_fees() {
fn failed_fee_transaction() {
let mut test = TemplateTest::new(["tests/templates/state"]);

let (account, owner_token, private_key) = test.create_owned_account();
let (account, owner_token, private_key) = test.create_funded_account();
let initial_balance: Amount =
test.call_method(account, "balance", args![CONFIDENTIAL_TARI_RESOURCE_ADDRESS], vec![]);

Expand Down Expand Up @@ -201,8 +201,8 @@ fn failed_fee_transaction() {
fn fail_partial_paid_fees() {
let mut test = TemplateTest::new(["tests/templates/state"]);

let (account, owner_token, private_key) = test.create_owned_account();
let (account2, owner_token2, _) = test.create_owned_account();
let (account, owner_token, private_key) = test.create_funded_account();
let (account2, owner_token2, _) = test.create_funded_account();
let orig_balance: Amount = test.call_method(account, "balance", args![CONFIDENTIAL_TARI_RESOURCE_ADDRESS], vec![]);
println!("{:?}", orig_balance);
test.enable_fees();
Expand Down Expand Up @@ -239,8 +239,8 @@ fn fail_partial_paid_fees() {
fn fail_pay_less_fees_than_fee_transaction() {
let mut test = TemplateTest::new(["tests/templates/state"]);

let (account, owner_token, private_key) = test.create_owned_account();
let (account2, owner_token2, _) = test.create_owned_account();
let (account, owner_token, private_key) = test.create_funded_account();
let (account2, owner_token2, _) = test.create_funded_account();
let orig_balance: Amount = test.call_method(account, "balance", args![CONFIDENTIAL_TARI_RESOURCE_ADDRESS], vec![]);
let state: ComponentAddress = test.call_function("State", "new", args![], vec![]);

Expand Down Expand Up @@ -302,8 +302,8 @@ fn fail_pay_less_fees_than_fee_transaction() {
fn fail_pay_too_little_no_fee_instruction() {
let mut test = TemplateTest::new(iter::empty::<&str>());

let (account, owner_token, private_key) = test.create_owned_account();
let (account2, owner_token2, _) = test.create_owned_account();
let (account, owner_token, private_key) = test.create_funded_account();
let (account2, owner_token2, _) = test.create_funded_account();
let orig_balance: Amount = test.call_method(account, "balance", args![CONFIDENTIAL_TARI_RESOURCE_ADDRESS], vec![]);

test.enable_fees();
Expand Down Expand Up @@ -336,8 +336,8 @@ fn fail_pay_too_little_no_fee_instruction() {
fn success_pay_fee_in_main_instructions() {
let mut test = TemplateTest::new(iter::empty::<&str>());

let (account, owner_token, private_key) = test.create_owned_account();
let (account2, owner_token2, _) = test.create_owned_account();
let (account, owner_token, private_key) = test.create_funded_account();
let (account2, owner_token2, _) = test.create_funded_account();
let orig_balance: Amount = test.call_method(account, "balance", args![CONFIDENTIAL_TARI_RESOURCE_ADDRESS], vec![]);

test.enable_fees();
Expand Down
10 changes: 5 additions & 5 deletions dan_layer/engine/tests/shenanigans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn it_rejects_unknown_substate_addresses() {
fn it_rejects_references_to_buckets_that_arent_in_scope() {
let mut test = TemplateTest::new(["tests/templates/shenanigans"]);
let template_addr = test.get_template_address("Shenanigans");
let (account, owner_token, owner_key) = test.create_owned_account();
let (account, owner_token, owner_key) = test.create_funded_account();

let result = test.execute_expect_success(
Transaction::builder()
Expand Down Expand Up @@ -170,7 +170,7 @@ fn it_rejects_double_ownership_of_vault() {
fn it_prevents_access_to_vault_id_in_component_context() {
let mut test = TemplateTest::new(["tests/templates/shenanigans"]);
let template_addr = test.get_template_address("Shenanigans");
let (account, _, _) = test.create_owned_account();
let (account, _, _) = test.create_funded_account();

let vault_id = {
let component = test.read_only_state_store().get_component(account).unwrap();
Expand Down Expand Up @@ -209,7 +209,7 @@ fn it_prevents_access_to_vault_id_in_component_context() {
fn it_prevents_access_to_out_of_scope_component() {
let mut test = TemplateTest::new(["tests/templates/shenanigans"]);
let template_addr = test.get_template_address("Shenanigans");
let (account, _, _) = test.create_owned_account();
let (account, _, _) = test.create_funded_account();

let result = test.execute_expect_success(
Transaction::builder()
Expand Down Expand Up @@ -242,7 +242,7 @@ fn it_prevents_access_to_out_of_scope_component() {
fn it_disallows_calls_on_vaults_that_are_not_owned_by_current_component() {
let mut test = TemplateTest::new(["tests/templates/shenanigans"]);
let template_addr = test.get_template_address("Shenanigans");
let (victim, _, _) = test.create_owned_account();
let (victim, _, _) = test.create_funded_account();
let (attacker, _, _) = test.create_empty_account();

let vault_id = {
Expand Down Expand Up @@ -274,7 +274,7 @@ fn it_disallows_calls_on_vaults_that_are_not_owned_by_current_component() {
fn it_disallows_vault_access_if_vault_is_not_owned() {
let mut test = TemplateTest::new(["tests/templates/shenanigans"]);
let template_addr = test.get_template_address("Shenanigans");
let (victim, _, _) = test.create_owned_account();
let (victim, _, _) = test.create_funded_account();

let vault_id = {
let component = test.read_only_state_store().get_component(victim).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion dan_layer/engine/tests/tariswap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn setup(fee: u16) -> TariSwapTest {

let (tariswap, lp_resource) = create_tariswap_component(&mut template_test, a_resource, b_resource, fee);

let (account_address, account_proof, _) = template_test.create_owned_account();
let (account_address, account_proof, _) = template_test.create_funded_account();
fund_account(&mut template_test, account_address, a_faucet);
fund_account(&mut template_test, account_address, b_faucet);

Expand Down
Loading

0 comments on commit 4b8382f

Please sign in to comment.