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.

Sign up for GitHub

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

Jump to bottom

Use the transaction builder to implement z_sendmany. #5458

Merged
merged 7 commits into from
Jan 12, 2022
Merged

Use the transaction builder to implement z_sendmany. #5458

merged 7 commits into from
Jan 12, 2022

Conversation

nuttycom
Copy link
Contributor

@nuttycom nuttycom commented Jan 9, 2022 •
edited by str4d
Loading

Fixes #5433

@nuttycom nuttycom force-pushed the cleanup/sendmany_txbuilder branch 2 times, most recently from 288f35e to c64aa47 Compare January 9, 2022 02:54
@nuttycom nuttycom marked this pull request as ready for review January 9, 2022 02:54
@nuttycom nuttycom added A-wallet Area: Wallet C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-rpc-interface Area: RPC interface A-wallet-change Area: Change handling within the wallet C-simplification Category: Changes that simplify the protocol specification or consensus rules. NU5 Network upgrade: NU5-specific tasks S-committed Status: Planned work in a sprint labels Jan 9, 2022
@nuttycom nuttycom added this to the Core Sprint 2021-52 milestone Jan 9, 2022
@nuttycom nuttycom force-pushed the cleanup/sendmany_txbuilder branch 5 times, most recently from 2459ab1 to 0454f0f Compare January 11, 2022 16:33
@nuttycom
Use transaction builder for asyncrpcoperation_sendmany.
f9477a4 
This replaces the old implementation of asyncrpcoperation_sendmany
with one where all transaction construction is delegated to the
transaction builder. The capabilities of z_sendmany are somewhat
modified in the process:

* z_sendmany now permits sending funds from a Sprout address to
  both transparent and Sapling addresses. PRIVACY NOTE: When
  user sends a Sprout->Sapling transaction, the amount of the
  transaction is publicly revealed.
* z_sendmany no longer supports transactions sending funds into
  the Sprout pool, with the exception of change amounts when
  sending from a Sprout address.
* When sending transparent coinbase funds to a set of shielded
  addresses, the amount sent to recipients must fully consume
  the input value and no change is permitted. This is a slightly
  weaker constraint than was previously implemented; in the past,
  only a single shielded recipient was allowed.
@nuttycom
Transaction builder must not set consensus branch ID for V4 transacti…
b54a63b 
…ons.
str4d
str4d requested changes Jan 12, 2022
View reviewed changes
qa/rpc-tests/wallet_listreceived.py Show resolved Hide resolved
src/wallet/rpcwallet.cpp Show resolved Hide resolved
src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved
src/wallet/rpcwallet.cpp Show resolved Hide resolved
src/wallet/rpcwallet.cpp Show resolved Hide resolved
src/wallet/asyncrpcoperation_sendmany.cpp Show resolved Hide resolved
src/wallet/asyncrpcoperation_sendmany.cpp Outdated Show resolved Hide resolved
src/wallet/asyncrpcoperation_sendmany.cpp Outdated Show resolved Hide resolved
src/wallet/asyncrpcoperation_sendmany.cpp Outdated Show resolved Hide resolved
src/wallet/test/rpc_wallet_tests.cpp Outdated Show resolved Hide resolved
@str4d
Copy link
Contributor

str4d commented Jan 12, 2022

CI is currently failing btest.

daira
daira previously approved these changes Jan 12, 2022
View reviewed changes
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK modulo comments.

@nuttycom
Fix conditions around dust thresholds.
322aee2
@nuttycom
Require an explicit flag to allow cross-pool transfers in z_sendmany.
9554ed2 
This adds an `allowRevealedAmounts` argument to z_sendmany. This
flag must be present to allow an amount-revealing cross-pool transfer
to be constructed.
@nuttycom nuttycom mentioned this pull request Jan 12, 2022
Closed
@nuttycom nuttycom force-pushed the cleanup/sendmany_txbuilder branch 2 times, most recently from ded07df to c90e857 Compare January 12, 2022 05:47
nuttycom and others added 3 commits January 12, 2022 10:45
@nuttycom nuttycom mentioned this pull request Jan 12, 2022
Closed
4 tasks
str4d
str4d approved these changes Jan 12, 2022
View reviewed changes
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK de58fc0

daira
daira approved these changes Jan 12, 2022
View reviewed changes
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK de58fc0

@str4d str4d merged commit 423489c into zcash:master Jan 12, 2022
@nuttycom nuttycom deleted the cleanup/sendmany_txbuilder branch January 12, 2022 23:51
LarryRuane pushed a commit to LarryRuane/zcash that referenced this pull request Jan 22, 2022
@str4d @LarryRuane
Merge pull request zcash#5458 from nuttycom/cleanup/sendmany_txbuilder
a6f9c9c 
Use the transaction builder to implement z_sendmany.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daira daira daira approved these changes

@str4d str4d str4d approved these changes

@LarryRuane LarryRuane Awaiting requested review from LarryRuane

Assignees
No one assigned
Labels
A-rpc-interface Area: RPC interface A-wallet Area: Wallet A-wallet-change Area: Change handling within the wallet C-cleanup Category: PRs that clean code up or issues documenting cleanup. C-simplification Category: Changes that simplify the protocol specification or consensus rules. NU5 Network upgrade: NU5-specific tasks S-committed Status: Planned work in a sprint
Projects
None yet
Milestone
Core Sprint 2021-52
3 participants
@nuttycom @str4d @daira