Skip to content

Commit

Permalink
zcash_client_backend: Track external addresses in inter-account trans…
Browse files Browse the repository at this point in the history 
…actions.

Previously, if the funding account for a received transaction output was
determined to be an account known to the wallet, the output was recorded
as though it were sent to an internal (change) address of the wallet.
  • Loading branch information
@nuttycom
nuttycom committed Mar 25, 2024
1 parent 404132b commit 151e6e5
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 37 deletions.
2 changes: 2 additions & 0 deletions zcash_client_backend/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ and this library adheres to Rust's notion of
feature flag.
- `zcash_client_backend::proto`:
- `ProposalDecodingError` has a new variant `TransparentMemo`.
- `zcash_client_backend::wallet::Recipient::InternalAccount` is now a structured
variant with an additional `external_address` field.
- `zcash_client_backend::zip321::render::amount_str` now takes a
`NonNegativeAmount` rather than a signed `Amount` as its argument.
- `zcash_client_backend::zip321::parse::parse_amount` now parses a
Expand Down
18 changes: 10 additions & 8 deletions zcash_client_backend/src/data_api/wallet.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1049,10 +1049,11 @@ where
memo.clone(),
)?;
sapling_output_meta.push((
Recipient::InternalAccount(
account,
PoolType::Shielded(ShieldedProtocol::Sapling),
),
Recipient::InternalAccount {
receiving_account: account,
external_address: None,
note: PoolType::Shielded(ShieldedProtocol::Sapling),
},
change_value.value(),
Some(memo),
))
Expand All @@ -1072,10 +1073,11 @@ where
memo.clone(),
)?;
orchard_output_meta.push((
Recipient::InternalAccount(
account,
PoolType::Shielded(ShieldedProtocol::Orchard),
),
Recipient::InternalAccount {
receiving_account: account,
external_address: None,
note: PoolType::Shielded(ShieldedProtocol::Orchard),

Check warning on line 1079 in zcash_client_backend/src/data_api/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api/wallet.rs#L1076-L1079 

Added lines #L1076 - L1079 were not covered by tests
},
change_value.value(),
Some(memo),
))
Expand Down
27 changes: 24 additions & 3 deletions zcash_client_backend/src/wallet.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//! light client.
use incrementalmerkletree::Position;
use zcash_keys::address::Address;
use zcash_note_encryption::EphemeralKeyBytes;
use zcash_primitives::{
consensus::BlockHeight,
Expand Down Expand Up @@ -70,7 +71,11 @@ pub enum Recipient<AccountId, N> {
Transparent(TransparentAddress),
Sapling(sapling::PaymentAddress),
Unified(UnifiedAddress, PoolType),
InternalAccount(AccountId, N),
InternalAccount {
receiving_account: AccountId,
external_address: Option<Address>,
note: N,
},
}

impl<AccountId, N> Recipient<AccountId, N> {
Expand All @@ -79,7 +84,15 @@ impl<AccountId, N> Recipient<AccountId, N> {
Recipient::Transparent(t) => Recipient::Transparent(t),
Recipient::Sapling(s) => Recipient::Sapling(s),
Recipient::Unified(u, p) => Recipient::Unified(u, p),
Recipient::InternalAccount(a, n) => Recipient::InternalAccount(a, f(n)),
Recipient::InternalAccount {

Check warning on line 87 in zcash_client_backend/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/wallet.rs#L87 

Added line #L87 was not covered by tests
receiving_account,
external_address,
note,
} => Recipient::InternalAccount {
receiving_account,
external_address,
note: f(note),
},
}
}
}
Expand All @@ -90,7 +103,15 @@ impl<AccountId, N> Recipient<AccountId, Option<N>> {
Recipient::Transparent(t) => Some(Recipient::Transparent(t)),
Recipient::Sapling(s) => Some(Recipient::Sapling(s)),
Recipient::Unified(u, p) => Some(Recipient::Unified(u, p)),
Recipient::InternalAccount(a, n) => n.map(|n0| Recipient::InternalAccount(a, n0)),
Recipient::InternalAccount {

Check warning on line 106 in zcash_client_backend/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/wallet.rs#L106 

Added line #L106 was not covered by tests
receiving_account,
external_address,
note,
} => note.map(|n0| Recipient::InternalAccount {
receiving_account,
external_address,
note: n0,
}),
}
}
}
Expand Down
65 changes: 42 additions & 23 deletions zcash_client_sqlite/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ use zcash_client_backend::{
wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput},
DecryptedOutput, PoolType, ShieldedProtocol, TransferType,
};
use zcash_keys::address::Address;
use zcash_primitives::{
block::BlockHash,
consensus::{self, BlockHeight},
Expand Down Expand Up @@ -1066,10 +1067,11 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
//TODO: Recover the UA, if possible.
Recipient::Sapling(output.note().recipient())
} else {
Recipient::InternalAccount(
*output.account(),
Note::Sapling(output.note().clone()),
)
Recipient::InternalAccount {
receiving_account: *output.account(),
external_address: None,
note: Note::Sapling(output.note().clone()),
}
};

wallet::put_sent_output(
Expand All @@ -1083,19 +1085,20 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
Some(output.memo()),
)?;

if matches!(recipient, Recipient::InternalAccount(_, _)) {
if matches!(recipient, Recipient::InternalAccount { .. }) {
wallet::sapling::put_received_note(wdb.conn.0, output, tx_ref, None)?;
}
}
TransferType::Incoming => {
wallet::sapling::put_received_note(wdb.conn.0, output, tx_ref, None)?;

if let Some(account_id) = funding_account {
// Even if the recipient address is external, record the send as internal.
let recipient = Recipient::InternalAccount(
*output.account(),
Note::Sapling(output.note().clone()),
);
let recipient = Recipient::InternalAccount {
receiving_account: *output.account(),

Check warning on line 1097 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1095-L1097 

Added lines #L1095 - L1097 were not covered by tests
// TODO: recover the actual UA, if possible
external_address: Some(Address::Sapling(output.note().recipient())),
note: Note::Sapling(output.note().clone()),

Check warning on line 1100 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1099-L1100 

Added lines #L1099 - L1100 were not covered by tests
};

wallet::put_sent_output(
wdb.conn.0,
Expand Down Expand Up @@ -1128,10 +1131,11 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
PoolType::Shielded(ShieldedProtocol::Orchard),

Check warning on line 1131 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1130-L1131 

Added lines #L1130 - L1131 were not covered by tests
)
} else {
Recipient::InternalAccount(
*output.account(),
Note::Orchard(*output.note()),
)
Recipient::InternalAccount {
receiving_account: *output.account(),
external_address: None,
note: Note::Orchard(*output.note()),

Check warning on line 1137 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1134-L1137 

Added lines #L1134 - L1137 were not covered by tests
}
};

wallet::put_sent_output(
Expand All @@ -1145,7 +1149,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
Some(output.memo()),
)?;

if matches!(recipient, Recipient::InternalAccount(_, _)) {
if matches!(recipient, Recipient::InternalAccount { .. }) {

Check warning on line 1152 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1152 

Added line #L1152 was not covered by tests
wallet::orchard::put_received_note(wdb.conn.0, output, tx_ref, None)?;
}
}
Expand All @@ -1154,10 +1158,17 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>

if let Some(account_id) = funding_account {

Check warning on line 1159 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1159 

Added line #L1159 was not covered by tests
// Even if the recipient address is external, record the send as internal.
let recipient = Recipient::InternalAccount(
*output.account(),
Note::Orchard(*output.note()),
);
let recipient = Recipient::InternalAccount {
receiving_account: *output.account(),

Check warning on line 1162 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1161-L1162 

Added lines #L1161 - L1162 were not covered by tests
// TODO: recover the actual UA, if possible
external_address: Some(Address::Unified(
UnifiedAddress::from_receivers(
Some(output.note().recipient()),
None,
None,
).expect("UA has an Orchard receiver by construction."))),
note: Note::Orchard(*output.note()),

Check warning on line 1170 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1164-L1170 

Added lines #L1164 - L1170 were not covered by tests
};

wallet::put_sent_output(
wdb.conn.0,
Expand Down Expand Up @@ -1288,13 +1299,17 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
)?;

match output.recipient() {
Recipient::InternalAccount(account, Note::Sapling(note)) => {
Recipient::InternalAccount {

Check warning on line 1302 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1302 

Added line #L1302 was not covered by tests
receiving_account,
note: Note::Sapling(note),
..
} => {
wallet::sapling::put_received_note(
wdb.conn.0,
&DecryptedOutput::new(
output.output_index(),
note.clone(),
*account,
*receiving_account,
output
.memo()
.map_or_else(MemoBytes::empty, |memo| memo.clone()),
Expand All @@ -1305,13 +1320,17 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
)?;
}
#[cfg(feature = "orchard")]
Recipient::InternalAccount(account, Note::Orchard(note)) => {
Recipient::InternalAccount {
receiving_account,
note: Note::Orchard(note),
..
} => {

Check warning on line 1327 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1323-L1327 

Added lines #L1323 - L1327 were not covered by tests
wallet::orchard::put_received_note(
wdb.conn.0,
&DecryptedOutput::new(
output.output_index(),
*note,
*account,
*receiving_account,

Check warning on line 1333 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L1333 

Added line #L1333 was not covered by tests
output
.memo()
.map_or_else(MemoBytes::empty, |memo| memo.clone()),
Expand Down
10 changes: 7 additions & 3 deletions zcash_client_sqlite/src/wallet.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2527,9 +2527,13 @@ fn recipient_params<P: consensus::Parameters>(
PoolType::Shielded(ShieldedProtocol::Sapling),
),
Recipient::Unified(addr, pool) => (Some(addr.encode(params)), None, *pool),
Recipient::InternalAccount(id, note) => (
None,
Some(id.to_owned()),
Recipient::InternalAccount {

Check warning on line 2530 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L2530 

Added line #L2530 was not covered by tests
receiving_account,
external_address,
note,
} => (
external_address.as_ref().map(|a| a.encode(params)),
Some(*receiving_account),
PoolType::Shielded(note.protocol()),
),
}
Expand Down

0 comments on commit 151e6e5

Please sign in to comment.