Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic put_block function for the memory wallet #1298

Merged
merged 17 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ bitvec = "1"
blake2s_simd = "1"
bls12_381 = "0.8"
jubjub = "0.10"
sapling = { package = "sapling-crypto", version = "0.1.2" }
#sapling = { package = "sapling-crypto", version = "0.1.2" }
sapling = { package = "sapling-crypto", git = "https://github.com/zcash/sapling-crypto", rev = "54fc7d3d9c8eeb8e6ef6f64619deed43b0b681a6" }
oxarbitrage marked this conversation as resolved.
Show resolved Hide resolved

# - Orchard
nonempty = "0.7"
Expand Down
4 changes: 2 additions & 2 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ pub struct ScannedBundles<NoteCommitment, NF> {
}

impl<NoteCommitment, NF> ScannedBundles<NoteCommitment, NF> {
pub(crate) fn new(
oxarbitrage marked this conversation as resolved.
Show resolved Hide resolved
pub fn new(
final_tree_size: u32,
commitments: Vec<(NoteCommitment, Retention<BlockHeight>)>,
nullifier_map: Vec<(TxId, u16, Vec<NF>)>,
Expand Down Expand Up @@ -1053,7 +1053,7 @@ pub struct ScannedBlock<A> {

impl<A> ScannedBlock<A> {
/// Constructs a new `ScannedBlock`
pub(crate) fn from_parts(
pub fn from_parts(
oxarbitrage marked this conversation as resolved.
Show resolved Hide resolved
block_height: BlockHeight,
block_hash: BlockHash,
block_time: u32,
Expand Down
86 changes: 76 additions & 10 deletions zcash_client_backend/src/data_api/mem_wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ use std::{
cmp::Ordering,
collections::{BTreeMap, HashMap, HashSet},
convert::Infallible,
num::NonZeroU32, hash::Hash,
hash::Hash,
num::NonZeroU32,
};
use zcash_keys::keys::{AddressGenerationError, DerivationError};
use zip32::{fingerprint::SeedFingerprint, DiversifierIndex};
use zip32::{fingerprint::SeedFingerprint, DiversifierIndex, Scope};

use zcash_primitives::{
block::BlockHash,
Expand All @@ -25,7 +26,7 @@ use zcash_protocol::{
use crate::{
address::UnifiedAddress,
keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
wallet::{NoteId, WalletTransparentOutput, WalletTx},
wallet::{NoteId, WalletSpend, WalletTransparentOutput, WalletTx},
};

use super::{
Expand Down Expand Up @@ -100,10 +101,7 @@ pub struct MemoryWalletDb {
}

impl MemoryWalletDb {
pub fn new(
network: Network,
max_checkpoints: usize
) -> Self {
pub fn new(network: Network, max_checkpoints: usize) -> Self {
Self {
network,
accounts: BTreeMap::new(),
Expand All @@ -122,6 +120,7 @@ impl MemoryWalletDb {
#[derive(Debug)]
pub enum Error {
AccountUnknown(u32),
ViewingKeyNotFound(u32),
MemoDecryption(memo::Error),
KeyDerivation(DerivationError),
AddressGeneration(AddressGenerationError),
Expand Down Expand Up @@ -236,7 +235,10 @@ impl WalletRead for MemoryWalletDb {
}

fn chain_height(&self) -> Result<Option<BlockHeight>, Self::Error> {
todo!()
match self.blocks.last_key_value() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to take into consideration here is how this is handled in zcash_client_sqlite: in that context, the chain tip may not yet have been scanned, so we get the current chain height as the maximum height known to the scan queue, rather than the blocks table.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocks are added to the memory wallet database by put_blocks which currently receives a vector of ScannedBlock. Do we plan to change that ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever is the case, ill open a separated PR for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocks are added to the memory wallet database by put_blocks which currently receives a vector of ScannedBlock. Do we plan to change that ?

No; the main distinction here is that the chain tip height is used in computing how to prioritize the scanning queue; we scan blocks out-of-order in many cases to improve time to spendability. The important related operation is update_chain_tip; basically, chain_height should return whatever is correct as of the last call to update_chain_tip, not the last call to put_blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In update_chain_tip where do you store the height that is passed by argument so you can use it in chain_height?

Some((last_key, _)) => Ok(Some(*last_key)),
None => Ok(None),
}
}

fn get_block_hash(&self, block_height: BlockHeight) -> Result<Option<BlockHash>, Self::Error> {
Expand Down Expand Up @@ -379,12 +381,76 @@ impl WalletWrite for MemoryWalletDb {
todo!()
}

/// Adds a sequence of blocks to the data store.
///
/// Assumes blocks will be here in order.
fn put_blocks(
&mut self,
// TODO: Figure out what to do with this field.
_from_state: &super::chain::ChainState,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, i added that at a4323c2

_blocks: Vec<ScannedBlock<Self::AccountId>>,
blocks: Vec<ScannedBlock<Self::AccountId>>,
) -> Result<(), Self::Error> {
todo!()
// TODO:
// - Make sure blocks are coming in order.
// - Make sure the first block in the sequence is tip + 1?
// - Add a check to make sure the blocks are not already in the data store.
for block in blocks.into_iter() {
let mut transactions = HashMap::new();
for transaction in block.transactions().into_iter().cloned() {
let txid = transaction.txid();
let account_id = 0; // TODO: Assuming the account is 0, handle this accordingly.
let ufvk = self
.accounts
.get(&account_id)
.ok_or(Error::AccountUnknown(0))?
.ufvk
.sapling()
.ok_or(Error::ViewingKeyNotFound(0))?;
let nk = ufvk.to_nk(Scope::External);

let spent_nullifiers = transaction
.sapling_outputs()
.iter()
.map(|o| {
let nullifier = o.note().nf(&nk, o.note_commitment_tree_position().into());
// TODO: Populate the bool field properly.
self.sapling_spends.entry(nullifier).or_insert((txid, true));
nullifier
})
.count();

// TODO: Add orchard nullifiers to the orchard spends.

// Is `self.tx_idx` field filled with all the transaction ids from the scanned blocks ?
self.tx_idx.insert(txid, block.block_height);
transactions.insert(txid, transaction);
}

let memory_block = MemoryWalletBlock {
height: block.block_height,
hash: block.block_hash,
block_time: block.block_time,
transactions,
// TODO: Add memos
memos: HashMap::new(),
};

nuttycom marked this conversation as resolved.
Show resolved Hide resolved
self.blocks.insert(block.block_height, memory_block);

// Add the sapling commitments to the sapling tree.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed now that we added the frontier?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; the frontier just provides a starting point, but the note commitments for each note in the block, and the commitment for the block-end checkpoint, must also be added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, i added the orchard version of this at 370a4aa

let sapling_block_commitments = block.into_commitments().sapling;
sapling_block_commitments.iter().map(|(node, height)| {
self.sapling_tree.append(*node, *height);
});

// TODO: Add orchard commitments to the orchard tree.

// TODO: Received notes need to be made available for note selection & balance calculation

// TODO: Spent notes need to be made unavailable for note selection & balance calculation
}

Ok(())
}

/// Adds a transparent UTXO received by the wallet to the data store.
Expand Down
3 changes: 3 additions & 0 deletions zcash_client_backend/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ impl<AccountId, N> Recipient<AccountId, Option<N>> {
/// The shielded subset of a [`Transaction`]'s data that is relevant to a particular wallet.
///
/// [`Transaction`]: zcash_primitives::transaction::Transaction
#[derive(Clone)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to do this for my version of put_blocks as i was not able to make it otherwise, it might be not needed and not wanted.

pub struct WalletTx<AccountId> {
txid: TxId,
block_index: usize,
Expand Down Expand Up @@ -227,6 +228,7 @@ impl transparent_fees::InputView for WalletTransparentOutput {
}

/// A reference to a spent note belonging to the wallet within a transaction.
#[derive(Clone)]
pub struct WalletSpend<Nf, AccountId> {
index: usize,
nf: Nf,
Expand Down Expand Up @@ -266,6 +268,7 @@ pub type WalletSaplingSpend<AccountId> = WalletSpend<sapling::Nullifier, Account
pub type WalletOrchardSpend<AccountId> = WalletSpend<orchard::note::Nullifier, AccountId>;

/// An output that was successfully decrypted in the process of wallet scanning.
#[derive(Clone)]
pub struct WalletOutput<Note, Nullifier, AccountId> {
index: usize,
ephemeral_key: EphemeralKeyBytes,
Expand Down