Skip to content

Commit

Permalink
fix(engine)!: allow multiple create components in one call (#1046)
Browse files Browse the repository at this point in the history
Description
---
- assigns a unique component address for each call to create_component 
- formalises component addresses using a public key 
- adds `.with_public_key_address` to component builder
- adds new test for multiple component creation
- use `with_public_key_address` in account template
- simplified the component address from public key derivation.

Motivation and Context
---

Previously you could not create multiple components in one call without
explicitly allocating an address.
This PR changes the default behaviour to generate a unique address,
thereby fixing the bug when creating multiple components in a single
call.
Some components need the address to be derived ahead of time using a
public key can do this by optionally specifying a public key address on
the component.

How Has This Been Tested?
---
New multiple component creation test. Existing account tests. Manually

What process can a PR reviewer use to test or verify this change?
---
Use a template that creates multiple components. Create an account that
does not exist for another public key.

Breaking Changes
---

- [ ] None
- [ ] Requires data directory to be deleted
- [x] Other - Please specify

BREAKING CHANGE: caller context calls have an additional `arg` field, so
templates using these need to be recompiled.
  • Loading branch information
sdbondi committed Jun 27, 2024
1 parent b98b1f8 commit 058a0a7
Show file tree
Hide file tree
Showing 20 changed files with 167 additions and 74 deletions.
6 changes: 3 additions & 3 deletions applications/tari_dan_wallet_daemon/src/handlers/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use tari_dan_wallet_sdk::{
};
use tari_dan_wallet_storage_sqlite::SqliteWalletStore;
use tari_engine_types::{
component::new_account_address_from_parts,
component::new_component_address_from_public_key,
confidential::ConfidentialClaim,
instruction::Instruction,
substate::{Substate, SubstateId},
Expand Down Expand Up @@ -834,7 +834,7 @@ fn get_or_create_account<T: WalletStore>(
.unwrap_or_else(|| sdk.key_manager_api().next_key(key_manager::TRANSACTION_BRANCH))?;
let account_pk = PublicKey::from_secret_key(&account_secret_key.key);

let account_address = new_account_address_from_parts(&ACCOUNT_TEMPLATE_ADDRESS, &account_pk);
let account_address = new_component_address_from_public_key(&ACCOUNT_TEMPLATE_ADDRESS, &account_pk);

// We have no involved substate addresses, so we need to add an output
(account_address.into(), account_secret_key, Some(name.to_string()))
Expand Down Expand Up @@ -882,7 +882,7 @@ pub async fn handle_transfer(
let mut fee_instructions = vec![];

let destination_account_address =
new_account_address_from_parts(&ACCOUNT_TEMPLATE_ADDRESS, &req.destination_public_key);
new_component_address_from_public_key(&ACCOUNT_TEMPLATE_ADDRESS, &req.destination_public_key);
let existing_account = sdk
.substate_api()
.scan_for_substate(&SubstateId::Component(destination_account_address), None)
Expand Down
43 changes: 34 additions & 9 deletions dan_layer/engine/src/runtime/impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,11 +471,16 @@ impl<TTemplateProvider: TemplateProvider<Template = LoadedTemplate>> RuntimeInte
self.tracker.lock_substate(&SubstateId::Component(*address), lock_flag)
}

fn caller_context_invoke(&self, action: CallerContextAction) -> Result<InvokeResult, RuntimeError> {
fn caller_context_invoke(
&self,
action: CallerContextAction,
args: EngineArgs,
) -> Result<InvokeResult, RuntimeError> {
self.invoke_modules_on_runtime_call("caller_context_invoke")?;

match action {
CallerContextAction::GetCallerPublicKey => {
args.assert_no_args("CallerContextAction::GetCallerPublicKey")?;
let sender_public_key =
RistrettoPublicKeyBytes::from_bytes(self.transaction_signer_public_key.as_bytes()).expect(
"RistrettoPublicKeyBytes::from_bytes should be infallible when called with RistrettoPublicKey \
Expand All @@ -485,15 +490,30 @@ impl<TTemplateProvider: TemplateProvider<Template = LoadedTemplate>> RuntimeInte
Ok(InvokeResult::encode(&sender_public_key)?)
},
CallerContextAction::GetComponentAddress => self.tracker.read_with(|state| {
args.assert_no_args("CallerContextAction::GetComponentAddress")?;
let call_frame = state.current_call_scope()?;
let maybe_address = call_frame
.get_current_component_lock()
.map(|l| l.address().as_component_address().unwrap());
Ok(InvokeResult::encode(&maybe_address)?)
}),
CallerContextAction::AllocateNewComponentAddress => self.tracker.write_with(|state| {
let public_key_address: Option<RistrettoPublicKeyBytes> = args.assert_one_arg()?;
let public_key_address = public_key_address
.map(|pk| {
RistrettoPublicKey::from_canonical_bytes(pk.as_bytes()).map_err(|_| {
RuntimeError::InvalidArgument {
argument: "public_key_address",
reason: "Invalid RistrettoPublicKeyBytes".to_string(),
}
})
})
.transpose()?;

let (template, _) = state.current_template()?;
let address = state.id_provider()?.new_component_address(*template, None)?;
let address = state
.id_provider()?
.new_component_address(*template, public_key_address)?;
let allocation = state.new_address_allocation(address)?;
Ok(InvokeResult::encode(&allocation)?)
}),
Expand Down Expand Up @@ -525,13 +545,18 @@ impl<TTemplateProvider: TemplateProvider<Template = LoadedTemplate>> RuntimeInte

match action {
ComponentAction::Create => {
let arg: CreateComponentArg = args.assert_one_arg()?;
let CreateComponentArg {
encoded_state,
owner_rule,
access_rules,
address_allocation,
} = args.assert_one_arg()?;

let template_addr = self.tracker.get_template_address()?;
let template_def = self.get_template_def(&template_addr)?;
validate_component_access_rule_methods(&arg.access_rules, &template_def)?;
validate_component_access_rule_methods(&access_rules, &template_def)?;

let owner_key = match arg.owner_rule {
let owner_key = match owner_rule {
OwnerRule::OwnedBySigner => {
Some(to_ristretto_public_key_bytes(&self.transaction_signer_public_key))
},
Expand All @@ -541,11 +566,11 @@ impl<TTemplateProvider: TemplateProvider<Template = LoadedTemplate>> RuntimeInte
};

let component_address = self.tracker.new_component(
arg.encoded_state,
encoded_state,
owner_key,
arg.owner_rule,
arg.access_rules,
arg.address_allocation,
owner_rule,
access_rules,
address_allocation,
)?;
Ok(InvokeResult::encode(&component_address)?)
},
Expand Down
6 changes: 5 additions & 1 deletion dan_layer/engine/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,11 @@ pub trait RuntimeInterface: Send + Sync {
fn reset_to_fee_checkpoint(&self) -> Result<(), RuntimeError>;
fn finalize(&self) -> Result<FinalizeResult, RuntimeError>;

fn caller_context_invoke(&self, action: CallerContextAction) -> Result<InvokeResult, RuntimeError>;
fn caller_context_invoke(
&self,
action: CallerContextAction,
args: EngineArgs,
) -> Result<InvokeResult, RuntimeError>;

fn call_invoke(&self, action: CallAction, args: EngineArgs) -> Result<InvokeResult, RuntimeError>;

Expand Down
4 changes: 1 addition & 3 deletions dan_layer/engine/src/runtime/tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ impl StateTracker {
addr.try_into()
.map_err(|address| RuntimeError::AddressAllocationTypeMismatch { address })?
},
None => state
.id_provider()?
.new_component_address(template_address, owner_key)?,
None => state.id_provider()?.new_component_address(template_address, None)?,
};

let component = ComponentBody { state: component_state };
Expand Down
7 changes: 4 additions & 3 deletions dan_layer/engine/src/transaction/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use tari_common_types::types::PublicKey;
use tari_dan_common_types::{services::template_provider::TemplateProvider, Epoch};
use tari_engine_types::{
commit_result::{ExecuteResult, FinalizeResult, RejectReason, TransactionResult},
component::new_component_address_from_public_key,
entity_id_provider::EntityIdProvider,
indexed_value::{IndexedValue, IndexedWellKnownTypes},
instruction::Instruction,
Expand Down Expand Up @@ -319,9 +320,9 @@ impl<TTemplateProvider: TemplateProvider<Template = LoadedTemplate> + 'static> T
}
})?;
let owner_pk = RistrettoPublicKeyBytes::from_bytes(owner_public_key.as_bytes()).unwrap();
let owner_token = NonFungibleAddress::from_public_key(owner_pk);
let account_address = new_component_address_from_public_key(&ACCOUNT_TEMPLATE_ADDRESS, owner_public_key);

let mut args = args![owner_token];
let mut args = args![NonFungibleAddress::from_public_key(owner_pk)];
if let Some(workspace_bucket) = workspace_bucket {
args.push(arg![Workspace(workspace_bucket)]);
}
Expand All @@ -336,7 +337,7 @@ impl<TTemplateProvider: TemplateProvider<Template = LoadedTemplate> + 'static> T
template_address: ACCOUNT_TEMPLATE_ADDRESS,
module_name: template.template_name().to_string(),
arg_scope,
entity_id: owner_pk.as_hash().leading_bytes().into(),
entity_id: account_address.entity_id(),
})?;

let result = Self::invoke_template(template, template_provider, runtime.clone(), function_def, args)?;
Expand Down
4 changes: 3 additions & 1 deletion dan_layer/engine/src/wasm/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ impl WasmProcess {
env.state().interface().consensus_invoke(arg.action)
}),
EngineOp::CallerContextInvoke => Self::handle(env, arg, |env, arg: CallerContextInvokeArg| {
env.state().interface().caller_context_invoke(arg.action)
env.state()
.interface()
.caller_context_invoke(arg.action, arg.args.into())
}),
EngineOp::GenerateRandomInvoke => Self::handle(env, arg, |env, arg: GenerateRandomInvokeArg| {
env.state().interface().generate_random_invoke(arg.action)
Expand Down
6 changes: 3 additions & 3 deletions dan_layer/engine/tests/templates/access_rules/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ mod access_rules_template {
pub fn with_auth_hook(allowed: bool, hook: String) -> Component<AccessRulesTest> {
let badges = create_badge_resource(AccessRule::DenyAll);

let address_alloc = CallerContext::allocate_component_address();
let address_alloc = CallerContext::allocate_component_address(None);

let tokens = ResourceBuilder::fungible()
.with_authorization_hook(*address_alloc.address(), hook)
Expand All @@ -93,7 +93,7 @@ mod access_rules_template {
pub fn with_auth_hook_attack_component(component_address: ComponentAddress) -> Component<AccessRulesTest> {
let badges = create_badge_resource(AccessRule::DenyAll);

let address_alloc = CallerContext::allocate_component_address();
let address_alloc = CallerContext::allocate_component_address(None);

let tokens = ResourceBuilder::fungible()
.with_authorization_hook(
Expand Down Expand Up @@ -185,7 +185,7 @@ mod access_rules_template {
pub fn resource_actions_restricted_to_component() -> Component<AccessRulesTest> {
let badges = create_badge_resource(AccessRule::AllowAll);

let allocation = CallerContext::allocate_component_address();
let allocation = CallerContext::allocate_component_address(None);
let tokens = ResourceBuilder::fungible()
.mintable(AccessRule::Restricted(RestrictedAccessRule::Require(
RequireRule::Require(allocation.address().clone().into()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mod template {

impl AddressAllocationTest {
pub fn create() -> (Component<Self>, ComponentAddress) {
let allocation = CallerContext::allocate_component_address();
let allocation = CallerContext::allocate_component_address(None);
let address = allocation.address().clone();
(
Component::new(Self {}).with_address_allocation(allocation).create(),
Expand All @@ -20,7 +20,7 @@ mod template {
}

pub fn drop_allocation() {
let _allocation = CallerContext::allocate_component_address();
let _allocation = CallerContext::allocate_component_address(None);
}
}
}
8 changes: 8 additions & 0 deletions dan_layer/engine/tests/templates/state/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ mod state_template {
.create()
}

pub fn create_multiple(n: u32) {
(0..n).for_each(|i| {
Component::new(Self { value: i })
.with_access_rules(AccessRules::new().default(AccessRule::AllowAll))
.create();
});
}

pub fn restricted() -> Component<Self> {
Component::new(Self { value: 0 })
.with_access_rules(
Expand Down
34 changes: 28 additions & 6 deletions dan_layer/engine/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,30 @@ fn test_state() {
assert_eq!(value, new_value);
}

#[test]
fn state_create_multiple_in_one_call() {
let mut template_test = TemplateTest::new(["tests/templates/state"]);
let store = template_test.read_only_state_store();

// constructor
template_test.call_function::<()>("State", "create_multiple", args![10u32], vec![]);

let template_address = template_test.get_template_address("State");
let mut count = 0usize;
store
.with_substates(|s| {
if s.substate_value()
.component()
.filter(|a| a.template_address == template_address)
.is_some()
{
count += 1;
}
})
.unwrap();
assert_eq!(count, 10);
}

#[test]
fn test_composed() {
let mut template_test = TemplateTest::new(vec!["tests/templates/state", "tests/templates/hello_world"]);
Expand All @@ -107,7 +131,7 @@ fn test_composed() {
.iter()
.map(|f| f.name.as_str())
.collect::<Vec<_>>();
assert_eq!(functions, vec!["new", "restricted", "set", "get"]);
assert_eq!(functions, vec!["new", "create_multiple", "restricted", "set", "get"]);

let component_state: ComponentAddress = template_test.call_function("State", "new", args![], vec![]);
let component_hw: ComponentAddress = template_test.call_function("HelloWorld", "new", args!["أهلا"], vec![]);
Expand All @@ -125,8 +149,6 @@ fn test_composed() {
assert_eq!(value, new_value);
}

// FIXME: this test breaks in CI but not in local
#[ignore]
#[test]
fn test_buggy_template() {
let err = compile_template("tests/templates/buggy", &["return_null_abi"])
Expand All @@ -135,7 +157,7 @@ fn test_buggy_template() {
.unwrap_err();
assert!(matches!(
err,
TemplateLoaderError::WasmModuleError(WasmExecutionError::MemoryPointerOutOfRange { .. })
TemplateLoaderError::WasmModuleError(WasmExecutionError::AbiDecodeError { .. })
));

let err = compile_template("tests/templates/buggy", &["unexpected_export_function"])
Expand Down Expand Up @@ -1334,7 +1356,7 @@ mod nft_indexes {

// TODO: these tests can be removed when create free test coins is removed
mod free_test_coins {
use tari_engine_types::component::new_account_address_from_parts;
use tari_engine_types::component::new_component_address_from_public_key;

use super::*;
#[test]
Expand All @@ -1346,7 +1368,7 @@ mod free_test_coins {

let owner_token = test.get_test_proof();
let future_account_component =
new_account_address_from_parts(&ACCOUNT_TEMPLATE_ADDRESS, test.get_test_public_key());
new_component_address_from_public_key(&ACCOUNT_TEMPLATE_ADDRESS, test.get_test_public_key());

test.execute_expect_success(
Transaction::builder()
Expand Down
18 changes: 8 additions & 10 deletions dan_layer/engine_types/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,9 @@ use tari_common_types::types::PublicKey;
use tari_template_lib::{
auth::{ComponentAccessRules, OwnerRule, Ownership},
crypto::RistrettoPublicKeyBytes,
models::{ComponentKey, EntityId, ObjectKey, TemplateAddress},
models::{EntityId, ObjectKey, TemplateAddress},
prelude::ComponentAddress,
Hash,
};
use tari_utilities::ByteArray;
#[cfg(feature = "ts")]
use ts_rs::TS;

Expand All @@ -42,17 +40,17 @@ use crate::{

/// Derives a component address.
///
/// This can be used to derive the component address from a public key if the component sets OwnerRule::OwnedBySigner or
/// OwnerRule::ByPublicKey
pub fn new_account_address_from_parts(template_address: &TemplateAddress, public_key: &PublicKey) -> ComponentAddress {
/// This can be used to derive the component address from a public key if the component sets public_key_address in the
/// component builder.
pub fn new_component_address_from_public_key(
template_address: &TemplateAddress,
public_key: &PublicKey,
) -> ComponentAddress {
let address = hasher32(EngineHashDomainLabel::ComponentAddress)
.chain(template_address)
.chain(public_key)
.result();
let key = ObjectKey::new(
EntityId::from_array(Hash::try_from(public_key.as_bytes()).unwrap().leading_bytes()),
ComponentKey::new(address.trailing_bytes()),
);
let key = ObjectKey::from_array(address.leading_bytes());
ComponentAddress::new(key)
}

Expand Down
Loading

0 comments on commit 058a0a7

Please sign in to comment.