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

Zip317 send all #1125

Merged
merged 26 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
114f888
uncomment send_all code
Oscar-Pepper May 23, 2024
c64929e
added proposal for total balance
Oscar-Pepper May 23, 2024
72287c7
added propose_send_all and test with no assertions
Oscar-Pepper May 23, 2024
1d16dd2
added helpers for calculating total fee and amounts for proposal
Oscar-Pepper May 24, 2024
7aa4da3
Merge branch 'add_transaction_request_mock_builder' into zip317_send_all
Oscar-Pepper May 24, 2024
8c7dd67
Merge branch 'add_transaction_request_mock_builder' into zip317_send_all
Oscar-Pepper May 24, 2024
50f8517
finished tests for helpers with payment pool error
Oscar-Pepper May 24, 2024
ff4e35e
Merge branch 'add_transaction_request_mock_builder' into zip317_send_all
Oscar-Pepper May 24, 2024
916cc7d
added dbgs
Oscar-Pepper May 24, 2024
ebed87a
failing test when dust is in the wallet
Oscar-Pepper May 27, 2024
0ecbe16
Merge branch 'dev' into zip317_send_all
Oscar-Pepper May 28, 2024
3b1b196
discovered bug where fee not calculated correctly when dust is in wallet
Oscar-Pepper May 28, 2024
b5e3f9c
change incorrect names
Oscar-Pepper May 30, 2024
ab9f4f3
resolve conflicts with dev
Oscar-Pepper May 30, 2024
8f47c84
fix doc warnings
Oscar-Pepper May 30, 2024
01f50b7
resolve merge conflicts with
Oscar-Pepper May 31, 2024
a5de60e
fixes to select notes
Oscar-Pepper May 31, 2024
05423c4
Merge branch 'dev' into zip317_send_all
Oscar-Pepper Jun 3, 2024
5f76abf
add assertions to test and remove debugs
Oscar-Pepper Jun 3, 2024
413c413
fix clippy warnings
Oscar-Pepper Jun 3, 2024
ea15e21
revert changes to note selection
Oscar-Pepper Jun 4, 2024
fcd453e
Merge branch 'fix_note_selection_bugs' into zip317_send_all
Oscar-Pepper Jun 4, 2024
b32b840
Merge branch 'fix_note_selection_bugs' into zip317_send_all
Oscar-Pepper Jun 4, 2024
6274aa7
added tests for fail cases and cleaned up error variants
Oscar-Pepper Jun 4, 2024
4db42c5
uncomment sendall from command list
Oscar-Pepper Jun 4, 2024
5ca4b2a
Merge branch 'dev' into zip317_send_all
Oscar-Pepper Jun 5, 2024
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
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions libtonode-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ zingoconfig = { path = "../zingoconfig" }

zcash_primitives = { workspace = true }
orchard = { workspace = true }
sapling-crypto = { workspace = true }
zcash_address = { workspace = true }
zcash_client_backend = { workspace = true }
shardtree = { workspace = true }
Expand Down
141 changes: 141 additions & 0 deletions libtonode-tests/tests/legacy.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
#![forbid(unsafe_code)]

use json::JsonValue;
use orchard::note_encryption::OrchardDomain;
use orchard::tree::MerkleHashOrchard;
use sapling_crypto::note_encryption::SaplingDomain;
use shardtree::store::memory::MemoryShardStore;
use shardtree::ShardTree;
use std::{fs::File, path::Path, time::Duration};
use zcash_address::unified::Fvk;
use zcash_client_backend::encoding::encode_payment_address;
use zcash_primitives::transaction::components::amount::NonNegativeAmount;
use zcash_primitives::zip339::Mnemonic;
use zcash_primitives::{
consensus::{BlockHeight, Parameters},
transaction::fees::zip317::MINIMUM_FEE,
};
use zingo_testutils::lightclient::from_inputs;
use zingo_testutils::{
self, build_fvk_client, check_client_balances, check_transaction_equality,
get_base_address_macro, increase_height_and_wait_for_client, paths::get_cargo_manifest_dir,
scenarios,
};
use zingolib::lightclient::propose::ProposeSendError;
use zingolib::utils::conversion::address_from_str;

use zingo_testvectors::{
block_rewards,
Expand Down Expand Up @@ -4244,3 +4250,138 @@ mod basic_transactions {
async fn proxy_server_worky() {
zingo_testutils::check_proxy_server_works().await
}

#[tokio::test]
async fn zip317_send_all() {
let (regtest_manager, _cph, faucet, recipient, _) =
scenarios::faucet_funded_recipient_default(100_000).await;

from_inputs::send(
&faucet,
vec![(&get_base_address_macro!(&recipient, "unified"), 5_000, None)],
)
.await
.unwrap();
increase_height_and_wait_for_client(&regtest_manager, &faucet, 1)
.await
.unwrap();
from_inputs::send(
&faucet,
vec![(
&get_base_address_macro!(&recipient, "sapling"),
50_000,
None,
)],
)
.await
.unwrap();
increase_height_and_wait_for_client(&regtest_manager, &faucet, 1)
.await
.unwrap();
from_inputs::send(
&faucet,
vec![(&get_base_address_macro!(&recipient, "sapling"), 4_000, None)],
)
.await
.unwrap();
increase_height_and_wait_for_client(&regtest_manager, &faucet, 1)
.await
.unwrap();
from_inputs::send(
&faucet,
vec![(&get_base_address_macro!(&recipient, "unified"), 4_000, None)],
)
.await
.unwrap();
increase_height_and_wait_for_client(&regtest_manager, &faucet, 1)
.await
.unwrap();
recipient.do_sync(false).await.unwrap();

recipient
.propose_send_all(
address_from_str(
&get_base_address_macro!(faucet, "sapling"),
&recipient.config().chain,
)
.unwrap(),
None,
)
.await
.unwrap();
recipient
.complete_and_broadcast_stored_proposal()
.await
.unwrap();
increase_height_and_wait_for_client(&regtest_manager, &recipient, 1)
.await
.unwrap();
faucet.do_sync(false).await.unwrap();

assert_eq!(
recipient
.wallet
.confirmed_balance_excluding_dust::<SaplingDomain>(None)
.await,
Some(0)
);
assert_eq!(
recipient
.wallet
.confirmed_balance_excluding_dust::<OrchardDomain>(None)
.await,
Some(0)
);
}

#[tokio::test]
async fn zip317_send_all_insufficient_funds() {
let (_regtest_manager, _cph, faucet, recipient, _) =
scenarios::faucet_funded_recipient_default(10_000).await;

let proposal_error = recipient
.propose_send_all(
address_from_str(
&get_base_address_macro!(faucet, "sapling"),
&recipient.config().chain,
)
.unwrap(),
None,
)
.await;

match proposal_error {
Err(ProposeSendError::Proposal(
zcash_client_backend::data_api::error::Error::InsufficientFunds {
available: a,
required: r,
},
)) => {
assert_eq!(a, NonNegativeAmount::const_from_u64(10_000));
assert_eq!(r, NonNegativeAmount::const_from_u64(20_000));
}
_ => panic!("expected an InsufficientFunds error"),
}
}

#[tokio::test]
async fn zip317_send_all_zero_value() {
let (_regtest_manager, _cph, faucet, recipient, _) =
scenarios::faucet_funded_recipient_default(10_000).await;

let proposal_error = recipient
.propose_send_all(
address_from_str(
&get_base_address_macro!(faucet, "unified"),
&recipient.config().chain,
)
.unwrap(),
None,
)
.await;

assert!(matches!(
proposal_error,
Err(ProposeSendError::ZeroValueSendAll)
))
}
35 changes: 21 additions & 14 deletions zingolib/src/commands.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! An interface that passes strings (e.g. from a cli, into zingolib)
//! upgrade-or-replace

use crate::data::proposal;
use crate::wallet::MemoDownloadOption;
use crate::{lightclient::LightClient, wallet};
use indoc::indoc;
Expand Down Expand Up @@ -892,11 +893,13 @@ impl Command for SendCommand {
}
};
RT.block_on(async move {
match lightclient
.propose_send(request)
.await {
match lightclient.propose_send(request).await {
zancas marked this conversation as resolved.
Show resolved Hide resolved
Ok(proposal) => {
object! { "fee" => proposal.steps().iter().fold(0, |acc, step| acc + u64::from(step.balance().fee_required())) }
let fee = match proposal::total_fee(&proposal) {
Ok(fee) => fee,
Err(e) => return object! { "error" => e.to_string() }.pretty(2),
};
object! { "fee" => fee.into_u64() }
}
Err(e) => {
object! { "error" => e.to_string() }
Expand All @@ -907,8 +910,6 @@ impl Command for SendCommand {
}
}

/*
// Unimplemented
#[cfg(feature = "zip317")]
struct SendAllCommand {}
#[cfg(feature = "zip317")]
Expand Down Expand Up @@ -947,24 +948,30 @@ impl Command for SendAllCommand {
}
};
RT.block_on(async move {
match lightclient
.propose_send_all(address, memo)
.await {
match lightclient.propose_send_all(address, memo).await {
Ok(proposal) => {
let amount = match proposal::total_payment_amount(&proposal) {
Ok(amount) => amount,
Err(e) => return object! { "error" => e.to_string() }.pretty(2),
};
let fee = match proposal::total_fee(&proposal) {
Ok(fee) => fee,
Err(e) => return object! { "error" => e.to_string() }.pretty(2),
};
object! {
"amount" => proposal.steps().iter().fold(0, |acc, step| acc + step.shielded_inputs().unwrap().notes().iter().fold(0, |acc, note| acc + u64::from(note.note().value()))),
"fee" => proposal.steps().iter().fold(0, |acc, step| acc + u64::from(step.balance().fee_required())),
"amount" => amount.into_u64(),
"fee" => fee.into_u64(),
}
}
Err(e) => {
object! { "error" => e }
object! { "error" => e.to_string() }
}
}
.pretty(2)
})
}
}
*/

#[cfg(feature = "zip317")]
struct QuickSendCommand {}
#[cfg(feature = "zip317")]
Expand Down Expand Up @@ -1762,7 +1769,7 @@ pub fn get_commands() -> HashMap<&'static str, Box<dyn Command>> {
}
#[cfg(feature = "zip317")]
{
//entries.push(("sendall", Box::new(SendAllCommand {})));
entries.push(("sendall", Box::new(SendAllCommand {})));
entries.push(("quicksend", Box::new(QuickSendCommand {})));
entries.push(("quickshield", Box::new(QuickShieldCommand {})));
entries.push(("confirm", Box::new(ConfirmCommand {})));
Expand Down
8 changes: 3 additions & 5 deletions zingolib/src/commands/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ pub(super) fn parse_send_args(args: &[&str], chain: &ChainType) -> Result<Receiv
Ok(send_args)
}

/*
// Parse the send arguments for `do_send` when sending all funds from shielded pools.
// Parse the send arguments for `propose_send` when sending all funds from shielded pools.
// The send arguments have two possible formats:
// - 1 argument in the form of a JSON string (single address only). '[{"address":"<address>", "memo":"<optional memo>"}]'
// - 2 (+1 optional) arguments for a single address send. &["<address>", "<optional memo>"]
Expand Down Expand Up @@ -156,7 +155,7 @@ pub(super) fn parse_send_all_args(

Ok((address, memo))
}
*/

// Checks send inputs do not contain memo's to transparent addresses.
fn check_memo_compatibility(
address: &Address,
Expand Down Expand Up @@ -420,7 +419,6 @@ mod tests {
}
}

/*
#[test]
#[cfg(feature = "zip317")]
fn parse_send_all_args() {
Expand Down Expand Up @@ -453,7 +451,7 @@ mod tests {
Err(CommandError::MultipleReceivers)
));
}
*/

#[test]
fn check_memo_compatibility() {
let chain = ChainType::Regtest(RegtestNetwork::all_upgrades_active());
Expand Down
56 changes: 55 additions & 1 deletion zingolib/src/data/proposal.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
//! The types of transaction Proposal that Zingo! uses.

use std::convert::Infallible;

use zcash_client_backend::proposal::Proposal;
use zcash_primitives::transaction::fees::zip317::FeeRule;
use zcash_primitives::transaction::{
components::amount::{BalanceError, NonNegativeAmount},
fees::zip317::FeeRule,
};

/// A proposed send to addresses.
/// Identifies the notes to spend by txid, pool, and output_index.
Expand All @@ -25,3 +29,53 @@ pub(crate) enum ZingoProposal {
#[allow(dead_code)] // TOdo construct it
Shield(ShieldProposal),
}

/// total sum of all transaction request payment amounts in a proposal
/// TODO: test for multi-step, zip320 currently unsupported.
pub fn total_payment_amount(
proposal: &TransferProposal,
) -> Result<NonNegativeAmount, BalanceError> {
proposal
.steps()
.iter()
.map(|step| step.transaction_request())
.try_fold(NonNegativeAmount::ZERO, |acc, request| {
(acc + request.total()?).ok_or(BalanceError::Overflow)
})
}

/// total sum of all fees in a proposal
/// TODO: test for multi-step, zip320 currently unsupported.
pub fn total_fee(proposal: &TransferProposal) -> Result<NonNegativeAmount, BalanceError> {
proposal
.steps()
.iter()
.map(|step| step.balance().fee_required())
.try_fold(NonNegativeAmount::ZERO, |acc, fee| {
(acc + fee).ok_or(BalanceError::Overflow)
})
}

#[cfg(test)]
mod tests {
use zcash_primitives::transaction::components::amount::NonNegativeAmount;

use crate::mocks;

#[test]
zancas marked this conversation as resolved.
Show resolved Hide resolved
fn total_payment_amount() {
let proposal = mocks::proposal::ProposalBuilder::default().build();
assert_eq!(
super::total_payment_amount(&proposal).unwrap(),
NonNegativeAmount::from_u64(100_000).unwrap()
);
}
#[test]
fn total_fee() {
let proposal = mocks::proposal::ProposalBuilder::default().build();
assert_eq!(
super::total_fee(&proposal).unwrap(),
NonNegativeAmount::from_u64(20_000).unwrap()
);
}
}
Loading
Loading