Skip to content

Commit

Permalink
fix(walletd): emit AccountChanged event on changes for account refresh (
Browse files Browse the repository at this point in the history
#956)

Description
---
Emit missing event if the account is updated via refresh.

Motivation and Context
---
The account monitor only emitted the AccountChanged event after a
successful transaction involving the account is detected. However, if a
client (e.g. cucumber) is polling the account refresh, the refresh can
detect changes to the account before the transaction is detected as
complete. This is the typical case as the cucumber polls faster than the
wallet polls the indexer for a transaction result.

This bug led to `Claim and transfer confidential assets via wallet
daemon` never finishing because once the finalised transaction comes
through, there are no changes detected because the refresh already
updated the account.

How Has This Been Tested?
---
`Claim and transfer confidential assets via wallet daemon` cucumber
completes.

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 committed Mar 6, 2024
1 parent 6d2b42b commit 0deb6cd
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 17 deletions.
43 changes: 28 additions & 15 deletions applications/tari_dan_wallet_daemon/src/services/account_monitor.rs
Expand Up @@ -185,14 +185,14 @@ where
.await
.optional()?;
let Some(ValidatorScanResult {
address: versioned_addr,
substate,
created_by_tx,
}) = scan_result
else {
warn!(target: LOG_TARGET, "Vault {} for account {} does not exist according to validator node", vault_addr, versioned_account_address);
continue;
};
address: versioned_addr,
substate,
created_by_tx,
}) = scan_result
else {
warn!(target: LOG_TARGET, "Vault {} for account {} does not exist according to validator node", vault_addr, versioned_account_address);
continue;
};

if let Some(vault_version) = maybe_vault_version {
// The first time a vault is found, know about the vault substate from the tx result but never added
Expand Down Expand Up @@ -227,7 +227,7 @@ where

async fn refresh_vault(
&self,
account_addr: &SubstateId,
account_address: &SubstateId,
vault: &Vault,
nfts: &HashMap<&NonFungibleId, &NonFungibleContainer>,
) -> Result<(), AccountMonitorError> {
Expand All @@ -236,46 +236,51 @@ where

let balance = vault.balance();
let vault_addr = SubstateId::Vault(*vault.vault_id());
if !accounts_api.exists_by_address(account_addr)? {
if !accounts_api.exists_by_address(account_address)? {
// This is not our account
return Ok(());
}

let mut has_changed = false;

if !accounts_api.has_vault(&vault_addr)? {
info!(
target: LOG_TARGET,
"🔒️ NEW vault {} in account {}",
vault.vault_id(),
account_addr
account_address
);
accounts_api.add_vault(
account_addr.clone(),
account_address.clone(),
vault_addr.clone(),
*vault.resource_address(),
vault.resource_type(),
// TODO: fetch the token symbol from the resource
None,
)?;
has_changed = true;
}

accounts_api.update_vault_balance(&vault_addr, balance)?;
info!(
target: LOG_TARGET,
"🔒️ vault {} in account {} has new balance {}",
vault.vault_id(),
account_addr,
account_address,
balance
);
if let Some(commitments) = vault.get_confidential_commitments() {
info!(
target: LOG_TARGET,
"🔒️ vault {} in account {} has {} confidential commitments",
vault.vault_id(),
account_addr,
account_address,
commitments.len()
);
self.wallet_sdk
.confidential_outputs_api()
.verify_and_update_confidential_outputs(account_addr, &vault_addr, commitments.values())?;
.verify_and_update_confidential_outputs(account_address, &vault_addr, commitments.values())?;
has_changed = true;
}

for id in vault.get_non_fungible_ids() {
Expand Down Expand Up @@ -316,7 +321,15 @@ where
};

non_fungibles_api.store_new_nft(&non_fungible)?;
has_changed = true;
}

if has_changed {
self.notify.notify(AccountChangedEvent {
account_address: account_address.clone(),
});
}

Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion dan_layer/consensus_tests/src/consensus.rs
Expand Up @@ -445,7 +445,7 @@ async fn foreign_block_distribution() {
let leaf1 = test.get_validator(&TestAddress::new("1")).get_leaf_block();
let leaf2 = test.get_validator(&TestAddress::new("4")).get_leaf_block();
let leaf3 = test.get_validator(&TestAddress::new("7")).get_leaf_block();
if leaf1.height > NodeHeight(20) || leaf2.height > NodeHeight(20) || leaf3.height > NodeHeight(20) {
if leaf1.height > NodeHeight(40) || leaf2.height > NodeHeight(40) || leaf3.height > NodeHeight(40) {
panic!(
"Not all transaction committed after {}/{}/{} blocks",
leaf1.height, leaf2.height, leaf3.height
Expand Down
29 changes: 28 additions & 1 deletion integration_tests/tests/log4rs/cucumber.yml
Expand Up @@ -80,10 +80,26 @@ appenders:
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [{X(node-public-key)},{X(node-id)}] {l:5} {m} // {f}:{L}{n}"

wallet_daemon:
kind: rolling_file
path: "{{log_dir}}/wallet_daemon.log"
policy:
kind: compound
trigger:
kind: size
limit: 10mb
roller:
kind: fixed_window
base: 1
count: 5
pattern: "log/validator-node/wallet_daemon.{}.log"
encoder:
pattern: "{d(%Y-%m-%d %H:%M:%S.%f)} [{t}] [{X(node-public-key)},{X(node-id)}] {l:5} {m} // {f}:{L}{n}"

# An appender named "other" that writes to a file with a custom pattern encoder
other:
kind: rolling_file
path: "log/validator-node/other.log"
path: "{{log_dir}}/other.log"
policy:
kind: compound
trigger:
Expand Down Expand Up @@ -121,6 +137,17 @@ loggers:
- dan_layer
additive: false

tari::dan::wallet_daemon:
level: debug
appenders:
- wallet_daemon
additive: false
tari::dan::wallet_sdk:
level: debug
appenders:
- wallet_daemon
additive: false

tari::dan:
level: debug
appenders:
Expand Down

0 comments on commit 0deb6cd

Please sign in to comment.