Differential D3308
Merge #9894: remove 'label' filter for rpc command help Authored by nakihito on Jun 13 2019, 00:03.
Details
6665977 remove 'label' filter for rpc command help (Gregory Sanders) Tree-SHA512: 0676c55b2893a469cd6785963affbb04126b9a32c130f1bb22dfd233ede6998f695187264e897ced4e0dac48451d9ae0311ebb4f7442096cad632fd22f75080e Backport of Core PR9894 make check test_runner.py
Diff Detail
Event TimelineComment Actions I tried backporting this one and I noticed that it produced duplicate entries for commands with "label" in the name, and I couldn't figure out why.... Is that still the case now? Can "examine bitcoin-cli help" be added to test plan? Comment Actions If this was happening to you, then it seems worth checking for. Will update test plan with this after I give it a shot and then re-up for review . Edit: I checked the output of bitcoin-cli help and did indeed see repeating commands. This looked weird to me, so after looking into the code a little deeper, I think there is a bug in the way the RPC commands are being registered to tableRPC. The duplicate commands seen in bitcoin-cli helpare the following wallet RPC commands: getlabeladdress getreceivedbylabel listreceivedbylabel setlabel For each of the above commands, there are two separate RPCs within rpc/wallet.cpp,'s ContextFreeRPCCommand (around line 4183) that share the same actor (function). I am currently looking into why this is happening. Comment Actions Investigate the duplicate RPCs in the help message. It sounds to me like some of the accounts code as either not deprecated correctly or needs to be cleaned up before this is backported. Comment Actions What I recall is that the new label commands that get added are sorted in correctly, and it's the existing label entries that are wrong. So we do want this Diff, and indeed some old code isn't cleaned up right. Comment Actions This was the state of Core when this backport was landed: https://github.com/instagibbs/bitcoin/blob/666597798c07d9df06681be63d2b2f51f0d5ca8a/src/wallet/rpcwallet.cpp
From what I can tell, the extra RPCs should not appear when calling ./bitcoin-cli help, but that is more because the extra account RPCs didn't exist in Core's code at the time. It also appears D1986 was one of the first steps to deprecating the account RPCs. https://github.com/bitcoin/bitcoin/pull/12953/files and https://github.com/bitcoin/bitcoin/pull/12892/files look to finish it and should fix the issue we are currently facing with this backport. Comment Actions As mentioned in the meeting I'm happy to green this if the duplicates are going to be removed reasonably soon in some other diffs, even if landed later on. Comment Actions Thanks. I'm gonna start working on the other two PRs so that they can be landed quickly after each other. I'll put this up for review again after I get those two up. Comment Actions I will green this once D3950 is getting close to completion. Right now, there are some missing backports so it's difficult to estimate a timeline. |