Change `WalletRead::get_transparent_receivers` signature

It needn't return the account id that was given as an input, and it shouldn't return an 11-byte diversifier index when a 31-bit child index is more appropriate.
This commit is contained in:
Andrew Arnott 2024-02-10 12:50:53 -07:00
parent 1733af8718
commit 918f5cc812
No known key found for this signature in database
GPG Key ID: 48F18646D6868924
12 changed files with 145 additions and 119 deletions

View File

@ -28,6 +28,7 @@ and this library adheres to Rust's notion of
- Arguments to `ScannedBlock::from_parts` have changed.
- Changes to the `WalletRead` trait:
- Added `get_orchard_nullifiers`
- Changed `get_transparent_receivers` return type.
- `ShieldedProtocol` has a new `Orchard` variant.
- `zcash_client_backend::fees`:
- Arguments to `ChangeStrategy::compute_balance` have changed.

View File

@ -14,7 +14,7 @@ use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
use zcash_primitives::{
block::BlockHash,
consensus::BlockHeight,
legacy::TransparentAddress,
legacy::{NonHardenedChildIndex, TransparentAddress},
memo::{Memo, MemoBytes},
transaction::{
components::amount::{Amount, BalanceError, NonNegativeAmount},
@ -24,7 +24,7 @@ use zcash_primitives::{
};
use crate::{
address::{AddressMetadata, UnifiedAddress},
address::UnifiedAddress,
decrypt::DecryptedOutput,
keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
proto::service::TreeState,
@ -560,7 +560,7 @@ pub trait WalletRead {
fn get_transparent_receivers(
&self,
account: AccountId,
) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error>;
) -> Result<HashMap<TransparentAddress, NonHardenedChildIndex>, Self::Error>;
/// Returns a mapping from transparent receiver to not-yet-shielded UTXO balance,
/// for each address associated with a nonzero balance.
@ -1109,14 +1109,14 @@ pub mod testing {
use zcash_primitives::{
block::BlockHash,
consensus::{BlockHeight, Network},
legacy::TransparentAddress,
legacy::{NonHardenedChildIndex, TransparentAddress},
memo::Memo,
transaction::{components::Amount, Transaction, TxId},
zip32::{AccountId, Scope},
};
use crate::{
address::{AddressMetadata, UnifiedAddress},
address::UnifiedAddress,
keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
wallet::{Note, NoteId, ReceivedNote, WalletTransparentOutput},
ShieldedProtocol,
@ -1284,7 +1284,7 @@ pub mod testing {
fn get_transparent_receivers(
&self,
_account: AccountId,
) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error> {
) -> Result<HashMap<TransparentAddress, NonHardenedChildIndex>, Self::Error> {
Ok(HashMap::new())
}

View File

@ -17,7 +17,7 @@ use crate::data_api::wallet::input_selection::InputSelectorError;
use crate::PoolType;
#[cfg(feature = "transparent-inputs")]
use zcash_primitives::{legacy::TransparentAddress, zip32::DiversifierIndex};
use zcash_primitives::legacy::TransparentAddress;
use crate::wallet::NoteId;
@ -70,9 +70,6 @@ pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError> {
#[cfg(feature = "transparent-inputs")]
AddressNotRecognized(TransparentAddress),
#[cfg(feature = "transparent-inputs")]
ChildIndexOutOfRange(DiversifierIndex),
}
impl<DE, CE, SE, FE> fmt::Display for Error<DE, CE, SE, FE>
@ -128,14 +125,6 @@ where
Error::AddressNotRecognized(_) => {
write!(f, "The specified transparent address was not recognized as belonging to the wallet.")
}
#[cfg(feature = "transparent-inputs")]
Error::ChildIndexOutOfRange(i) => {
write!(
f,
"The diversifier index {:?} is out of range for transparent addresses.",
i
)
}
}
}
}

View File

@ -639,17 +639,13 @@ where
for utxo in proposal.transparent_inputs() {
utxos.push(utxo.clone());
let diversifier_index = known_addrs
let child_index = known_addrs
.get(utxo.recipient_address())
.ok_or_else(|| Error::AddressNotRecognized(*utxo.recipient_address()))?
.diversifier_index();
let child_index = u32::try_from(*diversifier_index)
.map_err(|_| Error::ChildIndexOutOfRange(*diversifier_index))?;
.ok_or_else(|| Error::AddressNotRecognized(*utxo.recipient_address()))?;
let secret_key = usk
.transparent()
.derive_external_secret_key(child_index)
.derive_external_secret_key(*child_index)
.unwrap();
builder.add_transparent_input(

View File

@ -48,7 +48,7 @@ use shardtree::{error::ShardTreeError, ShardTree};
use zcash_primitives::{
block::BlockHash,
consensus::{self, BlockHeight},
legacy::TransparentAddress,
legacy::{NonHardenedChildIndex, TransparentAddress},
memo::{Memo, MemoBytes},
transaction::{
components::amount::{Amount, NonNegativeAmount},
@ -58,7 +58,7 @@ use zcash_primitives::{
};
use zcash_client_backend::{
address::{AddressMetadata, UnifiedAddress},
address::UnifiedAddress,
data_api::{
self,
chain::{BlockSource, CommitmentTreeRoot},
@ -346,7 +346,7 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
fn get_transparent_receivers(
&self,
_account: AccountId,
) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error> {
) -> Result<HashMap<TransparentAddress, NonHardenedChildIndex>, Self::Error> {
#[cfg(feature = "transparent-inputs")]
return wallet::get_transparent_receivers(self.conn.borrow(), &self.params, _account);

View File

@ -86,6 +86,9 @@ use zcash_primitives::{
zip32::{AccountId, DiversifierIndex},
};
#[cfg(feature = "transparent-inputs")]
use zcash_primitives::legacy::NonHardenedChildIndex;
use zcash_client_backend::{
address::{Address, UnifiedAddress},
data_api::{
@ -112,7 +115,7 @@ use {
crate::UtxoId,
rusqlite::Row,
std::collections::BTreeSet,
zcash_client_backend::{address::AddressMetadata, wallet::WalletTransparentOutput},
zcash_client_backend::wallet::WalletTransparentOutput,
zcash_primitives::{
legacy::{keys::IncomingViewingKey, Script, TransparentAddress},
transaction::components::{OutPoint, TxOut},
@ -349,7 +352,7 @@ pub(crate) fn get_transparent_receivers<P: consensus::Parameters>(
conn: &rusqlite::Connection,
params: &P,
account: AccountId,
) -> Result<HashMap<TransparentAddress, AddressMetadata>, SqliteClientError> {
) -> Result<HashMap<TransparentAddress, NonHardenedChildIndex>, SqliteClientError> {
let mut ret = HashMap::new();
// Get all UAs derived
@ -360,12 +363,12 @@ pub(crate) fn get_transparent_receivers<P: consensus::Parameters>(
while let Some(row) = rows.next()? {
let ua_str: String = row.get(0)?;
let di_vec: Vec<u8> = row.get(1)?;
let mut di_be: [u8; 11] = di_vec.try_into().map_err(|_| {
let mut di: [u8; 11] = di_vec.try_into().map_err(|_| {
SqliteClientError::CorruptedData(
"Diverisifier index is not an 11-byte value".to_owned(),
)
})?;
di_be.reverse();
di.reverse(); // BE -> LE conversion
let ua = Address::decode(params, &ua_str)
.ok_or_else(|| {
@ -380,16 +383,17 @@ pub(crate) fn get_transparent_receivers<P: consensus::Parameters>(
})?;
if let Some(taddr) = ua.transparent() {
ret.insert(
*taddr,
AddressMetadata::new(account, DiversifierIndex::from(di_be)),
);
let di_short = DiversifierIndex::from(di).try_into();
if let Ok(di_short) = di_short {
if let Some(index) = NonHardenedChildIndex::from_index(di_short) {
ret.insert(*taddr, index);
}
}
}
}
if let Some((taddr, diversifier_index)) = get_legacy_transparent_address(params, conn, account)?
{
ret.insert(taddr, AddressMetadata::new(account, diversifier_index));
if let Some((taddr, child_index)) = get_legacy_transparent_address(params, conn, account)? {
ret.insert(taddr, child_index);
}
Ok(ret)
@ -400,7 +404,7 @@ pub(crate) fn get_legacy_transparent_address<P: consensus::Parameters>(
params: &P,
conn: &rusqlite::Connection,
account: AccountId,
) -> Result<Option<(TransparentAddress, DiversifierIndex)>, SqliteClientError> {
) -> Result<Option<(TransparentAddress, NonHardenedChildIndex)>, SqliteClientError> {
// Get the UFVK for the account.
let ufvk_str: Option<String> = conn
.query_row(
@ -418,10 +422,7 @@ pub(crate) fn get_legacy_transparent_address<P: consensus::Parameters>(
ufvk.transparent()
.map(|tfvk| {
tfvk.derive_external_ivk()
.map(|tivk| {
let (taddr, child_index) = tivk.default_address();
(taddr, DiversifierIndex::from(child_index))
})
.map(|tivk| tivk.default_address())
.map_err(SqliteClientError::HdwalletError)
})
.transpose()

View File

@ -396,7 +396,9 @@ mod tests {
#[cfg(feature = "transparent-inputs")]
fn migrate_from_wm2() {
use zcash_client_backend::keys::UnifiedAddressRequest;
use zcash_primitives::transaction::components::amount::NonNegativeAmount;
use zcash_primitives::{
legacy::NonHardenedChildIndex, transaction::components::amount::NonNegativeAmount,
};
use crate::UA_TRANSPARENT;
@ -450,7 +452,7 @@ mod tests {
.and_then(|k| {
k.derive_external_ivk()
.ok()
.map(|k| k.derive_address(0).unwrap())
.map(|k| k.derive_address(NonHardenedChildIndex::ZERO).unwrap())
})
.map(|a| a.encode(&network));

View File

@ -292,13 +292,13 @@ mod tests {
use zcash_primitives::{
block::BlockHash,
consensus::{BlockHeight, Network, NetworkUpgrade, Parameters},
legacy::keys::IncomingViewingKey,
legacy::{keys::IncomingViewingKey, NonHardenedChildIndex},
memo::MemoBytes,
transaction::{
builder::{BuildConfig, BuildResult, Builder},
components::amount::NonNegativeAmount,
components::{amount::NonNegativeAmount, transparent},
fees::fixed,
},
transaction::{components::transparent, fees::fixed},
zip32::{AccountId, Scope},
};
use zcash_proofs::prover::LocalTxProver;
@ -354,7 +354,9 @@ mod tests {
);
builder
.add_transparent_input(
usk0.transparent().derive_external_secret_key(0).unwrap(),
usk0.transparent()
.derive_external_secret_key(NonHardenedChildIndex::ZERO)
.unwrap(),
transparent::OutPoint::new([1; 32], 0),
transparent::TxOut {
value: NonNegativeAmount::const_from_u64(EXTERNAL_VALUE + INTERNAL_VALUE),

View File

@ -7,6 +7,9 @@ use zcash_primitives::{
use crate::address::UnifiedAddress;
#[cfg(feature = "transparent-inputs")]
use zcash_primitives::legacy::NonHardenedChildIndex;
#[cfg(feature = "transparent-inputs")]
use {
std::convert::TryInto,
@ -68,13 +71,13 @@ pub mod sapling {
}
#[cfg(feature = "transparent-inputs")]
fn to_transparent_child_index(j: DiversifierIndex) -> Option<u32> {
fn to_transparent_child_index(j: DiversifierIndex) -> Option<NonHardenedChildIndex> {
let (low_4_bytes, rest) = j.as_bytes().split_at(4);
let transparent_j = u32::from_le_bytes(low_4_bytes.try_into().unwrap());
if transparent_j > (0x7FFFFFFF) || rest.iter().any(|b| b != &0) {
if rest.iter().any(|b| b != &0) {
None
} else {
Some(transparent_j)
NonHardenedChildIndex::from_index(transparent_j)
}
}
@ -388,7 +391,7 @@ impl UnifiedSpendingKey {
}
#[cfg(all(feature = "test-dependencies", feature = "transparent-inputs"))]
pub fn default_transparent_address(&self) -> (TransparentAddress, u32) {
pub fn default_transparent_address(&self) -> (TransparentAddress, NonHardenedChildIndex) {
self.transparent()
.to_account_pubkey()
.derive_external_ivk()
@ -812,13 +815,15 @@ mod tests {
#[cfg(feature = "transparent-inputs")]
#[test]
fn pk_to_taddr() {
use zcash_primitives::legacy::NonHardenedChildIndex;
let taddr =
legacy::keys::AccountPrivKey::from_seed(&MAIN_NETWORK, &seed(), AccountId::ZERO)
.unwrap()
.to_account_pubkey()
.derive_external_ivk()
.unwrap()
.derive_address(0)
.derive_address(NonHardenedChildIndex::ZERO)
.unwrap()
.encode(&MAIN_NETWORK);
assert_eq!(taddr, "t1PKtYdJJHhc3Pxowmznkg7vdTwnhEsCvR4".to_string());

View File

@ -6,6 +6,11 @@ use std::fmt;
use std::io::{self, Read, Write};
use std::ops::Shl;
#[cfg(feature = "transparent-inputs")]
use hdwallet::KeyIndex;
use subtle::{Choice, ConstantTimeEq};
use zcash_encoding::Vector;
#[cfg(feature = "transparent-inputs")]
@ -402,6 +407,61 @@ impl TransparentAddress {
}
}
/// A child index for a derived transparent address.
///
/// Only NON-hardened derivation is supported.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct NonHardenedChildIndex(u32);
impl ConstantTimeEq for NonHardenedChildIndex {
fn ct_eq(&self, other: &Self) -> Choice {
self.0.ct_eq(&other.0)
}
}
impl NonHardenedChildIndex {
pub const ZERO: NonHardenedChildIndex = NonHardenedChildIndex(0);
/// Parses the given ZIP 32 child index.
///
/// Returns `None` if the hardened bit is set.
pub fn from_index(i: u32) -> Option<Self> {
if i < (1 << 31) {
Some(NonHardenedChildIndex(i))
} else {
None
}
}
/// Returns the index as a 32-bit integer.
pub fn index(&self) -> u32 {
self.0
}
pub fn next(&self) -> Option<Self> {
Self::from_index(self.0 + 1)
}
}
#[cfg(feature = "transparent-inputs")]
impl TryFrom<KeyIndex> for NonHardenedChildIndex {
type Error = ();
fn try_from(value: KeyIndex) -> Result<Self, Self::Error> {
match value {
KeyIndex::Normal(i) => NonHardenedChildIndex::from_index(i).ok_or(()),
KeyIndex::Hardened(_) => Err(()),
}
}
}
#[cfg(feature = "transparent-inputs")]
impl From<NonHardenedChildIndex> for KeyIndex {
fn from(value: NonHardenedChildIndex) -> Self {
Self::Normal(value.index())
}
}
#[cfg(any(test, feature = "test-dependencies"))]
pub mod testing {
use proptest::prelude::{any, prop_compose};
@ -417,7 +477,7 @@ pub mod testing {
#[cfg(test)]
mod tests {
use super::{OpCode, Script, TransparentAddress};
use super::{NonHardenedChildIndex, OpCode, Script, TransparentAddress};
#[test]
fn script_opcode() {
@ -484,4 +544,21 @@ mod tests {
);
assert_eq!(addr.script().address(), Some(addr));
}
#[test]
fn nonhardened_indexes_accepted() {
assert_eq!(0, NonHardenedChildIndex::from_index(0).unwrap().index());
assert_eq!(
0x7fffffff,
NonHardenedChildIndex::from_index(0x7fffffff)
.unwrap()
.index()
);
}
#[test]
fn hardened_indexes_rejected() {
assert!(NonHardenedChildIndex::from_index(0x80000000).is_none());
assert!(NonHardenedChildIndex::from_index(0xffffffff).is_none());
}
}

View File

@ -6,14 +6,11 @@ use hdwallet::{
};
use secp256k1::PublicKey;
use sha2::{Digest, Sha256};
use subtle::{Choice, ConstantTimeEq};
use zcash_spec::PrfExpand;
use crate::{consensus, zip32::AccountId};
use super::TransparentAddress;
const MAX_TRANSPARENT_CHILD_INDEX: u32 = 0x7FFFFFFF;
use super::{NonHardenedChildIndex, TransparentAddress};
/// A [BIP44] private key at the account path level `m/44'/<coin_type>'/<account>'`.
///
@ -51,11 +48,11 @@ impl AccountPrivKey {
/// `m/44'/<coin_type>'/<account>'/0/<child_index>`.
pub fn derive_external_secret_key(
&self,
child_index: u32,
child_index: NonHardenedChildIndex,
) -> Result<secp256k1::SecretKey, hdwallet::error::Error> {
self.0
.derive_private_key(KeyIndex::Normal(0))?
.derive_private_key(KeyIndex::Normal(child_index))
.derive_private_key(child_index.into())
.map(|k| k.private_key)
}
@ -187,30 +184,31 @@ pub trait IncomingViewingKey: private::SealedChangeLevelKey + std::marker::Sized
#[allow(deprecated)]
fn derive_address(
&self,
child_index: u32,
child_index: NonHardenedChildIndex,
) -> Result<TransparentAddress, hdwallet::error::Error> {
let child_key = self
.extended_pubkey()
.derive_public_key(KeyIndex::Normal(child_index))?;
.derive_public_key(child_index.into())?;
Ok(pubkey_to_address(&child_key.public_key))
}
/// Searches the space of child indexes for an index that will
/// generate a valid transparent address, and returns the resulting
/// address and the index at which it was generated.
fn default_address(&self) -> (TransparentAddress, u32) {
let mut child_index = 0;
while child_index <= MAX_TRANSPARENT_CHILD_INDEX {
fn default_address(&self) -> (TransparentAddress, NonHardenedChildIndex) {
let mut child_index = NonHardenedChildIndex::ZERO;
loop {
match self.derive_address(child_index) {
Ok(addr) => {
return (addr, child_index);
}
Err(_) => {
child_index += 1;
child_index = child_index.next().unwrap_or_else(|| {
panic!("Exhausted child index space attempting to find a default address.");
});
}
}
}
panic!("Exhausted child index space attempting to find a default address.");
}
fn serialize(&self) -> Vec<u8> {
@ -292,39 +290,9 @@ impl ExternalOvk {
}
}
/// A child index for a derived transparent address.
///
/// Only NON-hardened derivation is supported.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct NonHardenedChildIndex(u32);
impl ConstantTimeEq for NonHardenedChildIndex {
fn ct_eq(&self, other: &Self) -> Choice {
self.0.ct_eq(&other.0)
}
}
impl NonHardenedChildIndex {
/// Parses the given ZIP 32 child index.
///
/// Returns `None` if the hardened bit is set.
pub fn from_index(i: u32) -> Option<Self> {
if i < (1 << 31) {
Some(NonHardenedChildIndex(i))
} else {
None
}
}
/// Returns the index as a 32-bit integer.
pub fn index(&self) -> u32 {
self.0
}
}
#[cfg(test)]
mod tests {
use super::{AccountPubKey, NonHardenedChildIndex};
use super::AccountPubKey;
#[test]
fn check_ovk_test_vectors() {
@ -571,21 +539,4 @@ mod tests {
assert_eq!(tv.external_ovk, external.as_bytes());
}
}
#[test]
fn nonhardened_indexes_accepted() {
assert_eq!(0, NonHardenedChildIndex::from_index(0).unwrap().index());
assert_eq!(
0x7fffffff,
NonHardenedChildIndex::from_index(0x7fffffff)
.unwrap()
.index()
);
}
#[test]
fn hardened_indexes_rejected() {
assert!(NonHardenedChildIndex::from_index(0x80000000).is_none());
assert!(NonHardenedChildIndex::from_index(0xffffffff).is_none());
}
}

View File

@ -950,6 +950,7 @@ mod tests {
#[cfg(feature = "transparent-inputs")]
fn binding_sig_absent_if_no_shielded_spend_or_output() {
use crate::consensus::NetworkUpgrade;
use crate::legacy::NonHardenedChildIndex;
use crate::transaction::builder::{self, TransparentBuilder};
let sapling_activation_height = TEST_NETWORK
@ -984,13 +985,14 @@ mod tests {
.to_account_pubkey()
.derive_external_ivk()
.unwrap()
.derive_address(0)
.derive_address(NonHardenedChildIndex::ZERO)
.unwrap()
.script(),
};
builder
.add_transparent_input(
tsk.derive_external_secret_key(0).unwrap(),
tsk.derive_external_secret_key(NonHardenedChildIndex::ZERO)
.unwrap(),
OutPoint::new([0u8; 32], 1),
prev_coin,
)