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.

Sign up for GitHub

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

Feature/support non fungibles in execution trace #2042

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
+208 −70
Open

Feature/support non fungibles in execution trace #2042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6a6ff5f
Support TakeNonFungibles in vault ops
lrubasze Dec 17, 2024
63b6283
Unsupport Recall from resource_changes in ExecutionTrace
lrubasze Dec 18, 2024
174b220
Test execution tracing for non-fungible resources transfers
lrubasze Dec 18, 2024
a4629ce
Some comments
lrubasze Dec 18, 2024
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
50 changes: 49 additions & 1 deletion radix-engine-tests/assets/blueprints/execution_trace/src/execution_trace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
use scrypto::prelude::*;

#[derive(ScryptoSbor, NonFungibleData)]
pub struct DummyData {}

#[blueprint]
mod execution_trace_test {
struct ExecutionTraceBp {
vault: Vault,
}

impl ExecutionTraceBp {
pub fn transfer_resource_between_two_components(
pub fn transfer_fungible_resource_between_two_components(
amount: u8,
use_take_advanced: bool,
) -> (
Expand Down Expand Up @@ -45,6 +48,41 @@ mod execution_trace_test {
(resource_address, source_component, target_component)
}

pub fn transfer_non_fungible_resource_between_two_components(
amount: u8,
) -> (
ResourceAddress,
Global<ExecutionTraceBp>,
Global<ExecutionTraceBp>,
) {
let mut entries = vec![];
for i in 1..amount as u64 + 1 {
entries.push((i.into(), DummyData {}));
}
let bucket = ResourceBuilder::new_integer_non_fungible::<DummyData>(OwnerRole::None)
.mint_initial_supply(entries);
let resource_address = bucket.resource_address();

let source_component = ExecutionTraceBp {
vault: Vault::with_bucket(bucket.into()),
}
.instantiate()
.prepare_to_globalize(OwnerRole::None)
.globalize();

let target_component = ExecutionTraceBp {
vault: Vault::new(resource_address),
}
.instantiate()
.prepare_to_globalize(OwnerRole::None)
.globalize();

let transfer_bucket: Bucket = source_component.take_non_fungibles(amount);
target_component.put(transfer_bucket);

(resource_address, source_component, target_component)
}

pub fn take(&mut self, amount: u8) -> Bucket {
self.vault.take(amount)
}
Expand All @@ -53,6 +91,16 @@ mod execution_trace_test {
self.vault.take_advanced(amount, WithdrawStrategy::Exact)
}

pub fn take_non_fungibles(&mut self, amount: u8) -> Bucket {
let amount = amount as u64;
let mut ids = indexset![];
for i in 1..amount + 1 {
ids.insert(NonFungibleLocalId::integer(i));
}

self.vault.as_non_fungible().take_non_fungibles(&ids).into()
}

pub fn put(&mut self, b: Bucket) {
self.vault.put(b)
}
Expand Down
202 changes: 142 additions & 60 deletions radix-engine-tests/tests/application/execution_trace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ use radix_transactions::{model::PreviewFlags, validation::TransactionValidator};
use scrypto_test::prelude::*;

#[test]
fn test_trace_resource_transfers_using_take() {
fn test_trace_fungible_resource_transfers_within_blueprint_using_take() {
run_resource_transfers_trace_test(false);
}

#[test]
fn test_trace_resource_transfers_using_take_advanced() {
fn test_trace_fungible_resource_transfers_within_blueprint_using_take_advanced() {
run_resource_transfers_trace_test(true);
}

Expand All @@ -29,7 +29,7 @@ fn run_resource_transfers_trace_test(use_take_advanced: bool) {
.call_function(
package_address,
"ExecutionTraceBp",
"transfer_resource_between_two_components",
"transfer_fungible_resource_between_two_components",
manifest_args!(transfer_amount, use_take_advanced),
)
.build();
Expand All @@ -47,87 +47,112 @@ fn run_resource_transfers_trace_test(use_take_advanced: bool) {
ComponentAddress,
) = receipt.expect_commit(true).output(1);

let resource_changes = &receipt
.expect_commit_success()
.execution_trace
.as_ref()
.unwrap()
.resource_changes;

/* There should be three resource changes: withdrawal from the source vault,
deposit to the target vault and withdrawal for the fee */
println!(
"{:?}",
receipt
.expect_commit_success()
.execution_trace
.as_ref()
.unwrap()
.resource_changes
);
assert_eq!(
2,
receipt
.expect_commit_success()
.execution_trace
.as_ref()
.unwrap()
.resource_changes
.len()
); // Two instructions
assert_eq!(
1,
receipt
.expect_commit_success()
.execution_trace
.as_ref()
.unwrap()
.resource_changes
.get(&0)
.unwrap()
.len()
); // One resource change in the first instruction (lock fee)
assert_eq!(
2,
receipt
.expect_commit_success()
.execution_trace
.as_ref()
.unwrap()
.resource_changes
.get(&1)
.unwrap()
.len()
); // One resource change in the first instruction (lock fee)
println!("{:?}", resource_changes);
assert_eq!(2, resource_changes.len()); // Two instructions
assert_eq!(1, resource_changes.get(&0).unwrap().len()); // One resource change in the first instruction (lock fee)
assert_eq!(2, resource_changes.get(&1).unwrap().len()); // Two resource changes in the second instruction (transfer_fungible_resource_between_two_components)

let fee_summary = receipt.fee_summary.clone();
let total_fee_paid = fee_summary.total_cost();

// Source vault withdrawal
assert!(receipt
.expect_commit_success()
.execution_trace
.as_ref()
.unwrap()
.resource_changes
assert!(resource_changes
.iter()
.flat_map(|(_, rc)| rc)
.any(|r| r.node_id == source_component.into()
&& r.amount == Decimal::from(transfer_amount).checked_neg().unwrap()));

// Target vault deposit
assert!(receipt
.expect_commit_success()
.execution_trace
.as_ref()
.unwrap()
.resource_changes
assert!(resource_changes
.iter()
.flat_map(|(_, rc)| rc)
.any(
|r| r.node_id == target_component.into() && r.amount == Decimal::from(transfer_amount)
));

// Fee withdrawal
assert!(receipt
assert!(resource_changes
.iter()
.flat_map(|(_, rc)| rc)
.any(|r| r.node_id == account.into()
&& r.amount == Decimal::from(total_fee_paid).checked_neg().unwrap()));
}

#[test]
fn test_trace_non_fungible_resource_transfers_within_blueprint() {
// Arrange
let mut ledger = LedgerSimulatorBuilder::new().build();
let (public_key, _, account) = ledger.new_allocated_account();
let package_address = ledger.publish_package_simple(PackageLoader::get("execution_trace"));
let transfer_amount = 10u8;

// Act
let manifest = ManifestBuilder::new()
.lock_fee(account, 500)
.call_function(
package_address,
"ExecutionTraceBp",
"transfer_non_fungible_resource_between_two_components",
manifest_args!(transfer_amount),
)
.build();
let receipt = ledger.preview_manifest(
manifest,
vec![public_key.clone().into()],
0,
PreviewFlags::default(),
);

// Assert
let (_resource_address, source_component, target_component): (
ResourceAddress,
ComponentAddress,
ComponentAddress,
) = receipt.expect_commit(true).output(1);

let resource_changes = &receipt
.expect_commit_success()
.execution_trace
.as_ref()
.unwrap()
.resource_changes
.resource_changes;

/* There should be three resource changes: withdrawal from the source vault,
deposit to the target vault and withdrawal for the fee */
println!("{:?}", resource_changes);
assert_eq!(2, resource_changes.len()); // Two instructions
assert_eq!(1, resource_changes.get(&0).unwrap().len()); // One resource change in the first instruction (lock fee)
assert_eq!(2, resource_changes.get(&1).unwrap().len()); // Two resource changes in the second instruction (transfer_non_fungible_resource_between_two_components)

let fee_summary = receipt.fee_summary.clone();
let total_fee_paid = fee_summary.total_cost();

// Source vault withdrawal
assert!(resource_changes
.iter()
.flat_map(|(_, rc)| rc)
.any(|r| r.node_id == source_component.into()
&& r.amount == Decimal::from(transfer_amount).checked_neg().unwrap()));

// Target vault deposit
assert!(resource_changes
.iter()
.flat_map(|(_, rc)| rc)
.any(
|r| r.node_id == target_component.into() && r.amount == Decimal::from(transfer_amount)
));

// Fee withdrawal
assert!(resource_changes
.iter()
.flat_map(|(_, rc)| rc)
.any(|r| r.node_id == account.into()
Expand Down Expand Up @@ -1047,3 +1072,60 @@ fn test_execution_trace_for_transaction_v2() {
)"#;
assert_eq!(format!("{:#?}", trace), expected_trace);
}

#[test]
fn test_trace_non_fungible_resource_transfers_using_manifest() {
// Arrange
let mut ledger = LedgerSimulatorBuilder::new().build();

let (from_public_key, _from_private_key, from_address) = ledger.new_allocated_account();
let (_to_public_key, _to_private_key, to_address) = ledger.new_allocated_account();

let nf_resource_address = ledger.create_non_fungible_resource_advanced(
NonFungibleResourceRoles::default(),
from_address,
4,
);
let nf_global_id =
NonFungibleGlobalId::new(nf_resource_address, NonFungibleLocalId::integer(1));

let manifest = ManifestBuilder::new()
.lock_fee(from_address, 100)
.withdraw_non_fungible_from_account(from_address, nf_global_id)
.take_all_from_worktop(nf_resource_address, "nf_bucket")
.try_deposit_or_abort(to_address, None, "nf_bucket")
.build();

let receipt = ledger.preview_manifest(
manifest,
vec![from_public_key.clone().into()],
0,
PreviewFlags::default(),
);
let fee_summary = receipt.fee_summary.clone();
let total_fee_paid = fee_summary.total_cost();

// Act & Assert: Execute the preview, followed by a normal execution.
let resource_changes = &receipt
.expect_commit_success()
.execution_trace
.as_ref()
.unwrap()
.resource_changes;

println!("{:?}", resource_changes);
assert!(resource_changes
.iter()
.flat_map(|(_, rc)| rc)
.any(|r| r.node_id == from_address.into() && r.amount == -total_fee_paid));

assert!(resource_changes
.iter()
.flat_map(|(_, rc)| rc)
.any(|r| r.node_id == from_address.into() && r.amount == dec!(-1)));

assert!(resource_changes
.iter()
.flat_map(|(_, rc)| rc)
.any(|r| r.node_id == to_address.into() && r.amount == dec!(1)));
}
26 changes: 17 additions & 9 deletions radix-engine/src/system/system_modules/execution_trace/module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,12 @@ impl From<&BucketSnapshot> for ResourceSpecifier {

#[derive(Debug, Clone)]
pub enum VaultOp {
Put(ResourceAddress, Decimal), // TODO: add non-fungible support
Put(ResourceAddress, Decimal),
Take(ResourceAddress, Decimal),
// We intentionally disregard IDs and only use the amount for non-fungibles.
// This maintains backward compatibility for users of the `ResourceChange` struct.
TakeNonFungibles(ResourceAddress, Decimal),
TakeAdvanced(ResourceAddress, Decimal),
Recall(ResourceAddress, Decimal),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth mentioning that Recall is not supported (and why)?
And perhaps (somewhere) that this feature isn't comprehensive/a good abstraction, and will be deprecated in the node's Core API at Dugong, with the intention to have it removed at a later point?

LockFee(Decimal, bool),
}

Expand Down Expand Up @@ -725,7 +727,9 @@ impl ExecutionTraceModule {
Actor::Method(actor @ MethodActor { node_id, ident, .. }) => {
if VaultUtil::is_vault_blueprint(&actor.get_blueprint_id()) {
match ident.as_str() {
VAULT_TAKE_IDENT | VAULT_TAKE_ADVANCED_IDENT | VAULT_RECALL_IDENT => {
VAULT_TAKE_IDENT
| VAULT_TAKE_ADVANCED_IDENT
| NON_FUNGIBLE_VAULT_TAKE_NON_FUNGIBLES_IDENT => {
for (_, resource) in &resource_summary.buckets {
let op = if ident == VAULT_TAKE_IDENT {
VaultOp::Take(resource.resource_address(), resource.amount())
Expand All @@ -734,8 +738,11 @@ impl ExecutionTraceModule {
resource.resource_address(),
resource.amount(),
)
} else if ident == VAULT_RECALL_IDENT {
VaultOp::Recall(resource.resource_address(), resource.amount())
} else if ident == NON_FUNGIBLE_VAULT_TAKE_NON_FUNGIBLES_IDENT {
VaultOp::TakeNonFungibles(
resource.resource_address(),
resource.amount(),
)
} else {
panic!("Unhandled vault method")
};
Expand All @@ -751,13 +758,14 @@ impl ExecutionTraceModule {
| VAULT_GET_AMOUNT_IDENT
| VAULT_FREEZE_IDENT
| VAULT_UNFREEZE_IDENT
| VAULT_BURN_IDENT => { /* no-op */ }
| VAULT_BURN_IDENT
| VAULT_RECALL_IDENT => { /* no-op */ }
FUNGIBLE_VAULT_LOCK_FEE_IDENT
| FUNGIBLE_VAULT_LOCK_FUNGIBLE_AMOUNT_IDENT
| FUNGIBLE_VAULT_UNLOCK_FUNGIBLE_AMOUNT_IDENT
| FUNGIBLE_VAULT_CREATE_PROOF_OF_AMOUNT_IDENT => { /* no-op */ }
NON_FUNGIBLE_VAULT_TAKE_NON_FUNGIBLES_IDENT
| NON_FUNGIBLE_VAULT_GET_NON_FUNGIBLE_LOCAL_IDS_IDENT

NON_FUNGIBLE_VAULT_GET_NON_FUNGIBLE_LOCAL_IDS_IDENT
| NON_FUNGIBLE_VAULT_CONTAINS_NON_FUNGIBLE_IDENT
| NON_FUNGIBLE_VAULT_RECALL_NON_FUNGIBLES_IDENT
| NON_FUNGIBLE_VAULT_CREATE_PROOF_OF_NON_FUNGIBLES_IDENT
Expand Down Expand Up @@ -904,7 +912,7 @@ pub fn calculate_resource_changes(
}
VaultOp::Take(resource_address, amount)
| VaultOp::TakeAdvanced(resource_address, amount)
| VaultOp::Recall(resource_address, amount) => {
| VaultOp::TakeNonFungibles(resource_address, amount) => {
let entry = &mut vault_changes
.entry(instruction_index)
.or_default()
Expand Down
Loading