feat(rpc): Implement `z_listunifiedreceivers` #6171

oxarbitrage · 2023-02-15T20:09:54Z

Motivation

We want to have z_listunifiedreceivers as part of the mining pool efforts.

Solution

Add support for z_listunifiedreceivers

Review

Anyone can review, there are some parts of the code that can be improved or refactored. Diff tests seems to work with several test vectors taken from zcash.

Reviewer Checklist

Will the PR name make sense to users?
- Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
Are the PR labels correct?
Does the code do what the ticket and PR says?
- Does it change concurrent code, unsafe code, or consensus rules?
How do you know it works? Does it have tests?

codecov · 2023-02-15T21:33:29Z

Codecov Report

Merging #6171 (680603c) into main (d413790) will increase coverage by 0.05%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6171      +/-   ##
==========================================
+ Coverage   77.93%   77.99%   +0.05%     
==========================================
  Files         304      304              
  Lines       39197    39197              
==========================================
+ Hits        30549    30572      +23     
+ Misses       8648     8625      -23

oxarbitrage · 2023-02-15T23:49:32Z

Some random addresses taken from https://github.com/zcash-hackworks/zcash-test-vectors/blob/master/test-vectors/zcash/unified_address.json#L39 in the zcash diff script:

$ ./zebra-utils/zcash-rpc-diff 6666 z_listunifiedreceivers u1l8xunezsvhq8fgzfl7404m450nwnd76zshscn6nfys7vyz2ywyh4cc5daaq0c7q2su5lqfh23sp7fkf3kt27ve5948mzpfdvckzaect2jtte308mkwlycj2u0eac077wu70vqcetkxf
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 6666) and zcashd (/media/oxarbitrage/4eb53770-4e4f-4d80-a830-0914f3f5b89a/zcash/zcash/src/zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

WARNING: comparing RPC responses from different heights:
zebrad is at: 1985400
zcashd is at: 1985332

Request:
z_listunifiedreceivers u1l8xunezsvhq8fgzfl7404m450nwnd76zshscn6nfys7vyz2ywyh4cc5daaq0c7q2su5lqfh23sp7fkf3kt27ve5948mzpfdvckzaect2jtte308mkwlycj2u0eac077wu70vqcetkxf

Querying zebrad main chain at height >=1985400...

real	0m0,002s
user	0m0,002s
sys	0m0,000s

Querying zcashd main chain at height >=1985332...

real	0m0,103s
user	0m0,002s
sys	0m0,000s


Response diff between zebrad and zcashd:
RPC responses were identical

/tmp/tmp.XEz5YAMEHY.rpc-diff/zebrad-main-1985400-z_listunifiedreceivers.json:
{
  "sapling": "zs1mrhc9y7jdh5r9ece8u5khgvj9kg0zgkxzdduyv0whkg7lkcrkx5xqem3e48avjq9wn2rukydkwn",
  "p2pkh": "t1V9mnyk5Z5cTNMCkLbaDwSskgJZucTLdgW"
}

$ ./zebra-utils/zcash-rpc-diff 6666 z_listunifiedreceivers u1snf9yr883aj2hm8pksp9aymnqdwzy42rpzuffevj35hhxeckays5pcpeq7vy2mtgzlcuc4mnh9443qnuyje0yx6h59angywka4v2ap6kchh2j96ezf9w0c0auyz3wwts2lx5gmk2sk9
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 6666) and zcashd (/media/oxarbitrage/4eb53770-4e4f-4d80-a830-0914f3f5b89a/zcash/zcash/src/zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

WARNING: comparing RPC responses from different heights:
zebrad is at: 1985425
zcashd is at: 1985366

Request:
z_listunifiedreceivers u1snf9yr883aj2hm8pksp9aymnqdwzy42rpzuffevj35hhxeckays5pcpeq7vy2mtgzlcuc4mnh9443qnuyje0yx6h59angywka4v2ap6kchh2j96ezf9w0c0auyz3wwts2lx5gmk2sk9

Querying zebrad main chain at height >=1985425...

real	0m0,002s
user	0m0,000s
sys	0m0,001s

Querying zcashd main chain at height >=1985366...

real	0m0,002s
user	0m0,002s
sys	0m0,000s


Response diff between zebrad and zcashd:
RPC responses were identical

/tmp/tmp.S0ia2ays4W.rpc-diff/zebrad-main-1985425-z_listunifiedreceivers.json:
{
  "orchard": "u1p5rt2l48lpmyyeypnffal288jgf2tz50fzn23jdp68vcujvwn0uz5u0evcp35jfmfvrl26az7suqqfm0skn9mmz9un86gqkq9va32v4s",
  "p2pkh": "t1WBxR5jNWgg4Cqeot3FvNkBb9ztYyjVELp"
}

$ ./zebra-utils/zcash-rpc-diff 6666 z_listunifiedreceivers u1en8ysypun4gdkdnu8zqqg6k73ankr9ffwfzg08wtzg9z939w0wupewemfrc8a630e8gc4uqucym0l4v44fszy3et4veyypt3jsyp0whfpfsn2lw30kj8nepe6wvvasf00wklh85u9v8glqndupmamk9z2ja9sanf70pp4yxvkt3dmyzxa0kkhv2c9pxmkghrxqk0590azvya3nzrtevj449nu3laskrhf7c7nj9cyw7ty38mccg4znrr876guu6pzndx7ngwzhmlsn8d89saf5araaacrhr9958xr6z23mj4qtzzn98whdpu8u7n8fhf5d2vypljda62q73du44sf0e0kxmq3gvgkta0qqgq9w6r403gc5jz2any02etmwlttkv84hgh95czhdf2jugk3u36ke0kchcthg240
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 6666) and zcashd (/media/oxarbitrage/4eb53770-4e4f-4d80-a830-0914f3f5b89a/zcash/zcash/src/zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

WARNING: comparing RPC responses from different heights:
zebrad is at: 1985425
zcashd is at: 1985377

Request:
z_listunifiedreceivers u1en8ysypun4gdkdnu8zqqg6k73ankr9ffwfzg08wtzg9z939w0wupewemfrc8a630e8gc4uqucym0l4v44fszy3et4veyypt3jsyp0whfpfsn2lw30kj8nepe6wvvasf00wklh85u9v8glqndupmamk9z2ja9sanf70pp4yxvkt3dmyzxa0kkhv2c9pxmkghrxqk0590azvya3nzrtevj449nu3laskrhf7c7nj9cyw7ty38mccg4znrr876guu6pzndx7ngwzhmlsn8d89saf5araaacrhr9958xr6z23mj4qtzzn98whdpu8u7n8fhf5d2vypljda62q73du44sf0e0kxmq3gvgkta0qqgq9w6r403gc5jz2any02etmwlttkv84hgh95czhdf2jugk3u36ke0kchcthg240

Querying zebrad main chain at height >=1985425...

real	0m0,005s
user	0m0,000s
sys	0m0,005s

Querying zcashd main chain at height >=1985377...

real	0m0,006s
user	0m0,005s
sys	0m0,000s


Response diff between zebrad and zcashd:
RPC responses were identical

/tmp/tmp.7T7s9y3cGU.rpc-diff/zebrad-main-1985425-z_listunifiedreceivers.json:
{
  "orchard": "u1sputg56gsjv23ct54mrwdlanzveqc7tzs5mxxnaa5cdegm03hg7x66aypyds65m92vt9uaxl672suhpc6z7htwvepyhkrpfusvazque9",
  "p2pkh": "t1JUTKCQAenX6MR3X6QiE1pDTBssXX7rWkT"
}

$ ./zebra-utils/zcash-rpc-diff 6666 z_listunifiedreceivers u1sem2gcey0emntrvxyjv8hyhq0w5fr4sxaj3cppgrfqgg6laydh8m78gy2cw2p54zzak3alnnsx4xjuhazpkrfcd90wl0c7ldj6y095hh5j6j2evry9vg5jqp4dyqpwqeryu7pes4sxyyyqwn6egs5daxk4473v9xpgzrwv5n0tvs93nlj4xpphq4vs2w8um9ph7zkte08t7fa509mnrt9apuhr22xq34mp2svjnq6rvfn0hg6lkehxtlj39vgjxjlkjfhx8rw2f02ckq8k5szcxsnhkgr2cqlmf2udl2gqdqr5t6
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 6666) and zcashd (/media/oxarbitrage/4eb53770-4e4f-4d80-a830-0914f3f5b89a/zcash/zcash/src/zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

WARNING: comparing RPC responses from different heights:
zebrad is at: 1985425
zcashd is at: 1985377

Request:
z_listunifiedreceivers u1sem2gcey0emntrvxyjv8hyhq0w5fr4sxaj3cppgrfqgg6laydh8m78gy2cw2p54zzak3alnnsx4xjuhazpkrfcd90wl0c7ldj6y095hh5j6j2evry9vg5jqp4dyqpwqeryu7pes4sxyyyqwn6egs5daxk4473v9xpgzrwv5n0tvs93nlj4xpphq4vs2w8um9ph7zkte08t7fa509mnrt9apuhr22xq34mp2svjnq6rvfn0hg6lkehxtlj39vgjxjlkjfhx8rw2f02ckq8k5szcxsnhkgr2cqlmf2udl2gqdqr5t6

Querying zebrad main chain at height >=1985425...

real	0m0,003s
user	0m0,002s
sys	0m0,000s

Querying zcashd main chain at height >=1985377...

real	0m0,006s
user	0m0,002s
sys	0m0,002s


Response diff between zebrad and zcashd:
RPC responses were identical

/tmp/tmp.egFKEkU0VJ.rpc-diff/zebrad-main-1985425-z_listunifiedreceivers.json:
{
  "sapling": "zs1mgn89sqs7umymah66jwa8xlqun2tuu7ytc3egj8ucwzuc6qff0ekmk7yasppndt8j424v4q6v88"
}
$ ./zebra-utils/zcash-rpc-diff 6666 z_listunifiedreceivers u1uehkuaq6rpfgt4ed5zpvhczg9apgpmyk5eq9qg23j8w7jxkhdnqzacte6gu8zgzfzgxy48ryzus3wnkhfxrxmlhs34xde3f34uxcnv3y6dsgj288vu56xs9f6ghvqsgkhuwtz4kkfxj8pa27v5p3ttlst340zvwx9nj6s0zw8p3wwk3zh37dwc7znqz52gj2fpaapzxzyagah0aeyxwa9fxxvyyj6w989v96ymsgf7s8s6ej9346p60fcjzzynvf9rmxevumdvt8l9mvhdfz4u5j4h7e0zjr2sde7fu7z9s02447qg6qzllm22egnx6ej6qczkkk2ygvpy08un9ggp853sddp6vskrlar6sygxec5f6c2t2eu9zmc728esy4sj9z853gxuplr6hw7lpcwzk20d85vuflnhlfv8nr3020r0v9z83ryudsyjv66rttxq2cscqlrdxakrmpjptzcf
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 6666) and zcashd (/media/oxarbitrage/4eb53770-4e4f-4d80-a830-0914f3f5b89a/zcash/zcash/src/zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

WARNING: comparing RPC responses from different heights:
zebrad is at: 1985425
zcashd is at: 1985397

Request:
z_listunifiedreceivers u1uehkuaq6rpfgt4ed5zpvhczg9apgpmyk5eq9qg23j8w7jxkhdnqzacte6gu8zgzfzgxy48ryzus3wnkhfxrxmlhs34xde3f34uxcnv3y6dsgj288vu56xs9f6ghvqsgkhuwtz4kkfxj8pa27v5p3ttlst340zvwx9nj6s0zw8p3wwk3zh37dwc7znqz52gj2fpaapzxzyagah0aeyxwa9fxxvyyj6w989v96ymsgf7s8s6ej9346p60fcjzzynvf9rmxevumdvt8l9mvhdfz4u5j4h7e0zjr2sde7fu7z9s02447qg6qzllm22egnx6ej6qczkkk2ygvpy08un9ggp853sddp6vskrlar6sygxec5f6c2t2eu9zmc728esy4sj9z853gxuplr6hw7lpcwzk20d85vuflnhlfv8nr3020r0v9z83ryudsyjv66rttxq2cscqlrdxakrmpjptzcf

Querying zebrad main chain at height >=1985425...

real	0m0,002s
user	0m0,002s
sys	0m0,000s

Querying zcashd main chain at height >=1985397...

real	0m0,006s
user	0m0,000s
sys	0m0,005s


Response diff between zebrad and zcashd:
RPC responses were identical

/tmp/tmp.zr9Syc6dHD.rpc-diff/zebrad-main-1985425-z_listunifiedreceivers.json:
{
  "orchard": "u14rlaearnd2sgca4nrzzhh030mck6cdqkxhuguuwcs39kqhvu7q2awz0t9gk2m6fx4p2qwsnnjwygm79qnla6lankkeqd58ej3stsgtnl"
}

$ ./zebra-utils/zcash-rpc-diff 6666 z_listunifiedreceivers u1rf4n5f682jspygln8r5pjwh6fmta7xz6n9x868f5wgc9prxsqkrh8jkpmn7wfnag56ml7czw68dv96299ft6s98p05u4jvdx3elyr83jqnzr603vw8yarptpg5pj73zlea0sksuje3r
Checking first node release info...
Checking second node release info...
Connected to zebrad (port 6666) and zcashd (/media/oxarbitrage/4eb53770-4e4f-4d80-a830-0914f3f5b89a/zcash/zcash/src/zcash-cli zcash.conf port).

Checking zebrad network and tip height...
Checking zcashd network and tip height...

WARNING: comparing RPC responses from different heights:
zebrad is at: 1985425
zcashd is at: 1985413

Request:
z_listunifiedreceivers u1rf4n5f682jspygln8r5pjwh6fmta7xz6n9x868f5wgc9prxsqkrh8jkpmn7wfnag56ml7czw68dv96299ft6s98p05u4jvdx3elyr83jqnzr603vw8yarptpg5pj73zlea0sksuje3r

Querying zebrad main chain at height >=1985425...

real	0m0,002s
user	0m0,001s
sys	0m0,000s

Querying zcashd main chain at height >=1985413...

real	0m0,002s
user	0m0,001s
sys	0m0,000s


Response diff between zebrad and zcashd:
RPC responses were identical

/tmp/tmp.RQTh2Ro9bl.rpc-diff/zebrad-main-1985425-z_listunifiedreceivers.json:
{
  "sapling": "zs1d466r22g5nnnpkemfwqkm0rasz6wkx7x3h56epase503k0nqdrn80zywzpdvwf7q6995jq63gce",
  "p2pkh": "t1McbDSFF4kWxbGdWhsw5P72YAeUJFxXcMJ"
}
$

teor2345

This looks great, and I'm happy it works!

We're adding a lot of new address types in this PR series. We're going to be using them for a long time, in a lot of code (RPCs and other address input code). So it might be worth spending some time designing an easy to use API.

I think we're trying to fight the design of zcash_address in this PR, rather than follow it. So that's why some of the code has a lot of conversions, and the error handling and formatting is tricky. (Or missing!)

If we follow their API, it will end up looking like this other address RPC:

zebra/zebra-rpc/src/methods/get_block_template_rpcs.rs

Lines 816 to 828 in 3f1d94d

    
           let Ok(address) = raw_address 
        
               .parse::<zcash_address::ZcashAddress>() else { 
        
                   return Ok(validate_address::Response::invalid()); 
        
               }; 
        
           let address = match address 
        
               .convert::<primitives::Address>() { 
        
                   Ok(address) => address, 
        
                   Err(err) => { 
        
                       tracing::debug!(?err, "conversion error"); 
        
                       return Ok(validate_address::Response::invalid()); 
        
                   } 
        
               };

I have some ideas about how we could design it. What do you think? Did you want to make these changes in this PR, or in another PR?

(@upbqdn you might want to check out this PR to make sure it doesn't have too many conflicts with ticket #6083.)

zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs

zebra-rpc/src/methods/get_block_template_rpcs/types/unified_address.rs

zebra-rpc/src/methods/get_block_template_rpcs.rs

zebra-chain/src/primitives/address.rs

zebra-rpc/src/methods/get_block_template_rpcs/types/unified_address.rs

zebra-rpc/src/methods/get_block_template_rpcs.rs

teor2345

This seems like a more reliable API.

zebra-chain/src/primitives/address.rs

zebra-rpc/src/methods/get_block_template_rpcs.rs

zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs

teor2345

Thanks, this is great, and thanks for your patience fixing this up!

I think there's just one required check left: our p2sh is the same as zcashd.

zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs

teor2345 · 2023-02-19T20:38:54Z

These are some optional changes we could make after PR #6185 merges:

unified payment address encoding implementation: feat(rpc): Implement z_listunifiedreceivers #6171 (comment)
move the unified to specific address conversions into the Address type: feat(rpc): Implement z_listunifiedreceivers #6171 (comment)

And one we can do at any time:

open a zcashd ticket with our new unified p2sh test vector: feat(rpc): Implement z_listunifiedreceivers #6171 (comment)

teor2345 · 2023-02-19T20:39:38Z

We did some CI failure fixes on the main branch, so I'm going to merge it in here.

teor2345 · 2023-02-19T20:39:47Z

@Mergifyio update

mergify · 2023-02-19T20:39:55Z

update

✅ Branch has been successfully updated

implement z_listunifiedreceivers

81712a9

github-actions bot added the C-feature Category: New features label Feb 15, 2023

add test vector

ddbd4c4

teor2345 mentioned this pull request Feb 16, 2023

Implement z_validateaddress RPC #6083

Closed

add snapshots

810ea84

oxarbitrage added A-rust Area: Updates to Rust code P-High 🔥 A-rpc Area: Remote Procedure Call interfaces labels Feb 16, 2023

oxarbitrage self-assigned this Feb 16, 2023

oxarbitrage marked this pull request as ready for review February 16, 2023 00:37

oxarbitrage requested review from a team as code owners February 16, 2023 00:37

oxarbitrage requested review from upbqdn and removed request for a team February 16, 2023 00:37

teor2345 reviewed Feb 16, 2023

View reviewed changes

oxarbitrage added 3 commits February 16, 2023 09:14

simplify sapling payment address

6795f76

send network argument to payment_address method

7b6ef5d

use expect for impossible address errors

835d532

teor2345 reviewed Feb 16, 2023

View reviewed changes

zebra-chain/src/primitives/address.rs Outdated Show resolved Hide resolved

remove network argument

6f3be8f

teor2345 mentioned this pull request Feb 17, 2023

feat(rpc): Implement the z_validateaddress RPC #6185

Merged

6 tasks

teor2345 reviewed Feb 17, 2023

View reviewed changes

zebra-chain/src/primitives/address.rs Outdated Show resolved Hide resolved

teor2345 reviewed Feb 17, 2023

View reviewed changes

zebra-chain/src/primitives/address.rs Outdated Show resolved Hide resolved

teor2345 reviewed Feb 17, 2023

View reviewed changes

zebra-chain/src/primitives/address.rs Show resolved Hide resolved

teor2345 reviewed Feb 17, 2023

View reviewed changes

zebra-rpc/src/methods/get_block_template_rpcs.rs Show resolved Hide resolved

oxarbitrage added 2 commits February 17, 2023 12:07

use already present network conversion

6a68a98

add additional snapshot

122e00d

teor2345 reviewed Feb 19, 2023

View reviewed changes

zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs Show resolved Hide resolved

teor2345 previously approved these changes Feb 19, 2023

View reviewed changes

zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs Show resolved Hide resolved

teor2345 dismissed their stale review via 37982dc February 19, 2023 20:36

Derive common traits on the RPC struct

37982dc

Merge branch 'main' into issue6030

680603c

teor2345 approved these changes Feb 19, 2023

View reviewed changes

mergify bot merged commit 31382d2 into main Feb 20, 2023

mergify bot deleted the issue6030 branch February 20, 2023 12:22

teor2345 mentioned this pull request Feb 20, 2023

Simplify z_listunifiedreceivers RPC implementation #6190

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rpc): Implement `z_listunifiedreceivers` #6171

feat(rpc): Implement `z_listunifiedreceivers` #6171

oxarbitrage commented Feb 15, 2023 •

edited

Loading

codecov bot commented Feb 15, 2023 •

edited

Loading

oxarbitrage commented Feb 15, 2023

teor2345 left a comment

teor2345 left a comment

teor2345 left a comment

teor2345 commented Feb 19, 2023

teor2345 commented Feb 19, 2023

teor2345 commented Feb 19, 2023

mergify bot commented Feb 19, 2023

	let Ok(address) = raw_address
	.parse::<zcash_address::ZcashAddress>() else {
	return Ok(validate_address::Response::invalid());
	};

	let address = match address
	.convert::<primitives::Address>() {
	Ok(address) => address,
	Err(err) => {
	tracing::debug!(?err, "conversion error");
	return Ok(validate_address::Response::invalid());
	}
	};

feat(rpc): Implement z_listunifiedreceivers #6171

feat(rpc): Implement z_listunifiedreceivers #6171

Conversation

oxarbitrage commented Feb 15, 2023 • edited Loading

Motivation

Solution

Review

Reviewer Checklist

codecov bot commented Feb 15, 2023 • edited Loading

Codecov Report

oxarbitrage commented Feb 15, 2023

teor2345 left a comment

Choose a reason for hiding this comment

teor2345 left a comment

Choose a reason for hiding this comment

teor2345 left a comment

Choose a reason for hiding this comment

teor2345 commented Feb 19, 2023

teor2345 commented Feb 19, 2023

teor2345 commented Feb 19, 2023

mergify bot commented Feb 19, 2023

✅ Branch has been successfully updated

feat(rpc): Implement `z_listunifiedreceivers` #6171

feat(rpc): Implement `z_listunifiedreceivers` #6171

oxarbitrage commented Feb 15, 2023 •

edited

Loading

codecov bot commented Feb 15, 2023 •

edited

Loading