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

Feature/wallet orchard UA RPCs #5300

Merged

str4d merged 3 commits into zcash:feature/wallet_unified_addresses from LarryRuane:feature/wallet_orchard

Dec 20, 2021

Collaborator

LarryRuane commented Sep 15, 2021 •

edited by str4d

Loading

Changes to wallet RPC code for orchard / NU5.

Part of #5177.
Part of #5178.
Part of #5180.
Part of #5181.
Part of #5182.
Part of #5183.
Part of #5191.

daira reviewed

View reviewed changes

src/rust/src/address_ffi.rs Outdated Show resolved Hide resolved

LarryRuane force-pushed the feature/wallet_orchard branch from 9b3843f to b3c7a41 Compare

September 15, 2021 14:45

Collaborator Author

LarryRuane commented Sep 15, 2021

Force-pushed to clean up ParseArbitraryInt() slightly, and add more tests.

r3ld3v added this to the Core Sprint 2021-36 milestone

LarryRuane force-pushed the feature/wallet_orchard branch from b3c7a41 to 294472d Compare

September 23, 2021 14:59

r3ld3v modified the milestones: Core Sprint 2021-36, Core Sprint 2021-38

daira reviewed

View reviewed changes

src/test/util_tests.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Show resolved Hide resolved

daira reviewed

View reviewed changes

Contributor

daira left a comment

Looks good so far, some minor comments.

r3ld3v modified the milestones: Core Sprint 2021-38, Core Sprint 2021-40

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira reviewed

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

daira approved these changes

View reviewed changes

Contributor

daira left a comment

utACK 44b0dc7e13e9741aca4033ac2e6b1c82e667ac06 with some more non-blocking suggestions.

Contributor

str4d commented Dec 14, 2021 •

edited

Loading

Blocking: Please rebase this PR to remove commit de1d41b, which should not be in the git history (it is the commit that was pushed directly to #5419, that I subsequently removed because it referred to a non-existent commit). This will also enable the most recent fixup commit (44b0dc7e13e9741aca4033ac2e6b1c82e667ac06) to be removed.

LarryRuane added 2 commits

December 14, 2021 14:42


          add ParseArbitraryInt() for diversifier index

d0b85b6


          add -orchardwallet experimental feature flag

cc70cd2

Also, temporarily don't allow -orchardwallet if running mainnet.

LarryRuane force-pushed the feature/wallet_orchard branch from 44b0dc7 to f21c0a4 Compare

December 14, 2021 21:53

Collaborator Author

LarryRuane commented Dec 14, 2021

Blocking: Please rebase this PR

Done, just force-pushed (to f21c0a49551a0abccd2efbe37f690e0d902c300f).

I'd like to do one more quick force-push for @daira's comments, they shouldn't take long.

LarryRuane force-pushed the feature/wallet_orchard branch from f21c0a4 to 296b743 Compare

December 14, 2021 22:21

Collaborator Author

LarryRuane commented Dec 14, 2021

Force-push (to 296b743cd6d397fc15ec3ce483d128806eeadcaa) to address Daira's review comments.

str4d mentioned this pull request

Deprecate old address-related RPCs #5177

Closed

str4d requested changes

View reviewed changes

src/test/util_tests.cpp Outdated Show resolved Hide resolved

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

src/wallet/rpcwallet.cpp

    
                          "\nResult:\n"

                          "{\n"

                          "  \"account\": n,       (numeric) next available account number\n"

                          "  \"unifiedaddress\"    (string) The default address for this account\n"

Contributor

str4d Dec 14, 2021

Does this need to be unifiedaddress (i.e. specifically indicating it is a Unified Address), or could it just be address (leaving the address kind defined by the string encoding)? If the latter, then we could instead make this defaultaddress to convey the documented semantics.

src/wallet/rpcwallet.cpp 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 Outdated Show resolved Hide resolved

src/wallet/test/rpc_wallet_tests.cpp Outdated Show resolved Hide resolved

src/wallet/rpcwallet.cpp Show resolved Hide resolved


          Add new and modify existing Orchard RPCs, non-functional

f139cdc

The new RPCs aren't functional, only have argument parsing and sample
outputs, guarded by experimental -orchardwallet flag.

These changes used the tickets linked from
zcash#5056 as a guide.

LarryRuane force-pushed the feature/wallet_orchard branch from 296b743 to f139cdc Compare

December 15, 2021 23:41

LarryRuane commented

View reviewed changes

Collaborator Author

LarryRuane left a comment

Force-pushed to address review comments

src/wallet/rpcwallet.cpp Show resolved Hide resolved

src/wallet/test/rpc_wallet_tests.cpp Outdated Show resolved Hide resolved

mdr0id added safe-to-build and removed safe-to-build labels

nuttycom self-requested a review

December 17, 2021 17:05

nuttycom approved these changes

View reviewed changes

Contributor

nuttycom left a comment

utACK f139cdc: Re-reviewed https://github.com/zcash/zcash/compare/ee71462755d3c269ec0e3ac8bd097b8c30910f3b..LarryRuane:feature/wallet_orchard

r3ld3v modified the milestones: Core Sprint 2021-48, Core Sprint 2021-50

r3ld3v added S-committed and removed S-committed labels

str4d approved these changes

View reviewed changes

Contributor

str4d left a comment

utACK f139cdc

str4d merged commit 3acc685 into zcash:feature/wallet_unified_addresses

str4d mentioned this pull request

Decide on naming of unifiedaddress in new RPC outputs #5432

Closed

str4d mentioned this pull request

Implement wallet UA RPCs #5475

Merged

str4d mentioned this pull request

Add Unified Address support to the zcashd wallet #5419

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-committed safe-to-build