Skip to content

Commit

Permalink
fix(engine): remove unnecessary allocations from GetNonFungibleIds (#861
Browse files Browse the repository at this point in the history
)

Description
---
Remove unnecessary allocation when calling
`VaultAction::GetNonFungibleIds` and `BucketAction::GetNonFungibleIds`

Motivation and Context
---

Previously, when we were using borsh encoding, `BTreeSet`s would not
deserialize in the WASM (unfortunately, no specific details in my code
comment). Since the change to use CBOR this is no longer the case, which
allows us to encode a reference to the BTreeSet without the
into_iter/collect conversion to a Vec. Since Sets and Vecs are
represented in the same CBOR binary format, we can deserialize from a
BTreeSet to a Vec.

It is important to remove any unnecessary contract execution resource
usage.

How Has This Been Tested?
---
Modified existing test to call the bucket get_non_fungible_ids method.
Vault get_non_fungible is already tested.

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

Breaking Changes
---

- [x] None
- [ ] Requires data directory to be deleted
- [ ] Other - Please specify
  • Loading branch information
sdbondi authored Jan 4, 2024
1 parent bf566dc commit 3d09615
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 11 deletions.
10 changes: 2 additions & 8 deletions dan_layer/engine/src/runtime/impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -887,12 +887,7 @@ impl<TTemplateProvider: TemplateProvider<Template = LoadedTemplate>> RuntimeInte

self.tracker.write_with(|state| {
let vault_lock = state.lock_substate(&SubstateAddress::Vault(vault_id), LockFlag::Read)?;
// NOTE: A BTreeSet does not decode when received in the WASM
let non_fungible_ids = state
.get_vault(&vault_lock)?
.get_non_fungible_ids()
.iter()
.collect::<Vec<_>>();
let non_fungible_ids = state.get_vault(&vault_lock)?.get_non_fungible_ids();
let result = InvokeResult::encode(&non_fungible_ids)?;
state.unlock_substate(vault_lock)?;
Ok(result)
Expand Down Expand Up @@ -1238,8 +1233,7 @@ impl<TTemplateProvider: TemplateProvider<Template = LoadedTemplate>> RuntimeInte

self.tracker.write_with(|state| {
let bucket = state.get_bucket(bucket_id)?;
let non_fungible_ids = bucket.non_fungible_ids().iter().collect::<Vec<_>>();
Ok(InvokeResult::encode(&non_fungible_ids)?)
Ok(InvokeResult::encode(bucket.non_fungible_ids())?)
})
},
}
Expand Down
17 changes: 16 additions & 1 deletion dan_layer/engine/tests/account_nfts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
// SPDX-License-Identifier: BSD-3-Clause

use tari_engine_types::instruction::Instruction;
use tari_template_lib::{args, models::ComponentAddress, prelude::Metadata, resource::TOKEN_SYMBOL};
use tari_template_lib::{
args,
models::{ComponentAddress, NonFungibleId},
prelude::Metadata,
resource::TOKEN_SYMBOL,
};
use tari_template_test_tooling::TemplateTest;

#[test]
Expand Down Expand Up @@ -57,6 +62,11 @@ fn basic_nft_mint() {
Instruction::PutLastInstructionOutputOnWorkspace {
key: b"my_nft".to_vec(),
},
Instruction::CallFunction {
template_address: account_nft_template_test.get_template_address("Account"),
function: "get_non_fungible_ids_for_bucket".to_string(),
args: args![Variable("my_nft")],
},
Instruction::CallMethod {
component_address: owner_component_address,
method: "deposit".to_string(),
Expand All @@ -68,4 +78,9 @@ fn basic_nft_mint() {
.unwrap();

assert!(result.finalize.result.is_accept());

let bucket_nfts = result.finalize.execution_results[2]
.decode::<Vec<NonFungibleId>>()
.unwrap();
assert_eq!(bucket_nfts.len(), 1);
}
3 changes: 1 addition & 2 deletions dan_layer/engine_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ tari_template_lib = { workspace = true }
tari_utilities = { workspace = true }

borsh = { workspace = true }
# if we set this version in the workspace it would break other crates
# if we set this version in the workspace it would break other crates
base64 = "0.21.0"
blake2 = { workspace = true }
rand = { workspace = true }
Expand All @@ -27,7 +27,6 @@ serde = { workspace = true, default-features = true }
serde_json = { workspace = true }
thiserror = { workspace = true }


[features]
default = ["debugging"]
# Includes the ability to create free test coins
Expand Down
7 changes: 7 additions & 0 deletions dan_layer/template_builtin/templates/account/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use tari_template_lib::prelude::*;
#[template]
mod account_template {
use super::*;

pub struct Account {
// TODO: Lazy key value map/store
vaults: BTreeMap<ResourceAddress, Vault>,
Expand Down Expand Up @@ -234,5 +235,11 @@ mod account_template {
let v = self.get_vault_mut(resource);
v.create_proof_by_amount(amount)
}

/// Utility function to allow bucket NFT content to be inspected. An empty vec is returned if the bucket does
/// not contain any NFTs or does not contain a NonFungible resource.
pub fn get_non_fungible_ids_for_bucket(bucket: Bucket) -> Vec<NonFungibleId> {
bucket.get_non_fungible_ids()
}
}
}

0 comments on commit 3d09615

Please sign in to comment.