Details

Reviewers

majcosta
Fabien

Group Reviewers

Restricted Project

Commits

rABC7a6b18076c65: [avalanche] add a decodeavalancheproof RPC

Summary

I need this RPC to write tests and investigate test failures. It will probably be useful as well to help users troubleshoot proofs that they generated.

Depends on D9525 for Proof::fromHex

Test Plan

src/bitcoin-cli help decodeavalancheproof
src/bitcoin-cli decodeavalancheproof "0b000000000000000c0000000000000021030b4c866585dd868a9d62348a9cd008d6a312937048fff31670e7e920cfc7a7440105c5f72f5d6da3085583e75ee79340eb4eff208c89988e7ed0efb30b87298fa30000000000f2052a0100000003000000210227d85ba011276cf25b51df6a188b75e604b38770a462b2d0e9fb2fc839ef5d3faf07f001dd38e9b4a43d07d5d449cc0f7d2888d96b82962b3ce516d1083c0e031773487fc3c4f2e38acd1db9741321b91a79b82d1c2cfd47793261e4ba003cf5"

test/functional/test_runner.py abc_rpc_ava*

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 15727
Build 31358: Build Diff	lint-circular-dependencies · build-debug · build-without-wallet · build-diff · build-clang · build-clang-tidy
Build 31357: arc lint + arc unit

Event Timeline

PiRK created this revision.May 14 2021, 09:47

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 14 2021, 09:47

PiRK requested review of this revision.May 14 2021, 09:47

Harbormaster completed remote builds in B15720: Diff 28481.May 14 2021, 10:01

PiRK added a child revision: D9527: [avalanche] remove proof details from getavalanchepeerinfo RPC.May 14 2021, 10:11

PiRK retitled this revision from [avalanche] implement decodeavalancheproof RPC to [avalanche] add a decodeavalancheproof RPC.May 14 2021, 10:12

Need to factor the proof deserialization code with D9525

rebase on D9525, use Proof::FromHex

Harbormaster completed remote builds in B15725: Diff 28486.May 14 2021, 11:41

majcosta requested changes to this revision.May 14 2021, 21:59

majcosta added a subscriber: majcosta.

majcosta added inline comments.

src/rpc/avalanche.cpp
266–268 ↗	(On Diff #28486)	this isn't necessary

This revision now requires changes to proceed.May 14 2021, 21:59

g_avalanche is already initialized when calling a RPC command
And this specific RPC does not need the processor anyway.

Harbormaster completed remote builds in B15727: Diff 28489.May 15 2021, 08:27

majcosta accepted this revision.May 15 2021, 17:30

This revision is now accepted and ready to land.May 15 2021, 17:30

fix wrong proofid

Apparently, ToHex(proof.getId()) does not do what I expect it to do. Use proof.getId().toString() instead. Fix the test accordingly.

Also, use assert_equal in the tests.

remove g_avalanche check in the right RPC command.

Harbormaster completed remote builds in B15736: Diff 28505.May 17 2021, 12:59

Harbormaster completed remote builds in B15737: Diff 28506.May 17 2021, 13:05

use ToString for other uint256 (txid) for consistency. Use a const reference for 2 variables in the loop.

Harbormaster completed remote builds in B15738: Diff 28507.May 17 2021, 14:13

Fabien requested changes to this revision.May 17 2021, 15:15

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/rpc/avalanche.cpp
279 ↗	(On Diff #28507)	uint64_t seems more appropriated.
284 ↗	(On Diff #28507)	I think most signatures are printed with base64 encoding, at least this is the case with messages signatures and PSBT. We should probably do the same for consistency.