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.

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

Bitcoind Feature Parity #1546

Open
jakesylvestre opened this issue Mar 6, 2020 · 16 comments
Open

Bitcoind Feature Parity #1546

jakesylvestre opened this issue Mar 6, 2020 · 16 comments

Comments

@jakesylvestre
Copy link
Collaborator

jakesylvestre commented Mar 6, 2020

Hi,

So I decided to open this after the discussion we had around #1545 regarding releases and around how versioning is based around compatibility with RPC. In addition to what's below, I'd eventually like some documentation around fields that are just wrong/outdated so we can get going on those. I've put this together for getmempoolentry in #1522 and it's probably a pretty good place to reference and see what I'm shooting for. I'll also add that there are two levels of compatibility for each method rpcclient and rpcserver. The former is far easier to do and defines a template for the latter (see #1524) for reference.

Would love to hear @jcvernaleo /@Roasbeef's thoughts on this, but it might make sense to establish some standard priority around the rpcclient methods since they're so easy to fix/add. As far as tagging, I'd suggest the following for these PRs going forward:

  • RPC Parity (low risk)
  • Server Parity (high risk)

There's a few automated ways to figure this out we can look at in the mean time. Bitcoind has an awesome python based test framework I might be able to write a wrapper for to test btcd methods, but that'll require more investigation.

*Note: for convenience, I used rpcserver to check for methods since unimplemented tend to be there. In cases where they're not (I don't know of any) this will be inaccurate. Just add a comment and I'll fix it) This doesn't work and I didn't end up doing this, there are a bunch of methods missing rpcunimplemented, this should be fixed

Note: A bunch of these probably are as simple as a command name change (e.g. account->label in wallet rpc's). These are included anyway

X marks a feature is complete.

As per request this list has been moved to the wiki

Blockchain

Method Server Client PR (Client) PR (Server) Issue
getbestblockhash x x
getblock x x
getblockchaininfo x x #1630/(client was out of date, see #1670
getblockcount x x
getblockfilter
getblockhash x x
getblockheader x x
getblockstats x #1630
getchaintips x
getchaintxstats x
getdifficulty x x
getmempoolancestors
getmempooldescendants
getmempoolentry x #1522
getmempoolinfo x x #1586
getrawmempool x x
gettxout x x
gettxoutproof
gettxoutsetinfo x
preciousblock x
pruneblockchain
savemempool
scantxoutset
verifychain x x
verifytxoutproof x

Control

Method Server Client PR (Client) PR (Server) Issue
getmemoryinfo
getrpcinfo
help x x
logging
stop x x
uptime x x

Mining

Method Server Client PR (Client) PR (Server) Issue
getblocktemplate x x
getmininginfo x x
getnetworkhashps x x
prioritisetransaction
submitblock x x
submitheader

Network

Method Server Client PR (Client) PR (Server) Issue
addnode x x
clearbanned
disconnectnode
getaddednodeinfo x x This is implemented, but appears to have a bug fixed by #800
getconnectioncount x x
getnettotals x x
getnetworkinfo x x
getnodeaddresses #1590 #1590
getpeerinfo x x
listbanned
ping x x
setban
setnetworkactive

Raw Transactions

Method Server Client PR (Client) PR (Server) Issue
analyzepsbt
combinepsbt
combinerawtransaction
converttopsbt
createpsbt
createrawtransaction x x
decodepsbt
decoderawtransaction x x
decodescript x x
finalizepsbt
fundrawtransaction x
getrawtransaction x x
joinpsbts
sendrawtransaction x
signrawtransactionwithkey
testmempoolaccept x x
utxoupdatepsbt x x

Util

Method Server Client PR (Client) PR (Server) Issue
createmultisig
deriveaddresses x
estimatesmartfee x
getdescriptorinfo #1578
signmessagewithprivkey #1585
validateaddress x x
verifymessage x x

Address

Note: server is not applicable here since that's in a different repo, I've also left Issue blank on these since issues are scattered across repos. I've left the column here so we can add in as we go

Method Client PR (Client) Issue
abandontransaction
abortrescan
addmultisigaddress x
backupwallet x
bumpfee
createwallet
dumpprivkey x
dumpwallet x
encryptwallet x
getaddressesbylabel
getaddressinfo x
getbalance x
getbalances x
getnewaddress x #1567
getrawchangeaddress x
getreceivedbyaddress x
getreceivedbylabel
gettransaction x
getunconfirmedbalance x
getwalletinfo x #1638
importaddress
importmulti #1584
importprivkey x
importprunedfunds
importpubkey
importwallet x
keypoolrefill x
listaddressgroupings x
listlabels
listlockunspent x
listreceivedbyaddress x
listreceivedbylabel x
listsinceblock x
listtransactions x
listunspent x
listwalletdir
listwallets
loadwallet
lockunspent x
removeprunedfunds
rescanblockchain
sendmany x
sendtoaddress x
sethdseed
setlabel
settxfee x
setwalletflag
signmessage x
signrawtransactionwithwallet #1561
unloadwallet
walletcreatefundedpsbt #1573 #1596
walletlock x
walletpassphrase x
walletpassphrasechange x
walletprocesspsbt #1573 #1596

zmq

_Note: server is not applicable here since btcd does not implement zmq

Method Client PR (Client) Issue
getzmqnotifications #1587
@jakesylvestre jakesylvestre changed the title [WIP] Bitcoind Feature Parity Bitcoind Feature Parity Mar 6, 2020
@jcvernaleo
Copy link
Member

@jakesyl really nice work here.

I haven't looked at the bitcoin python tests in a while (maybe a year or two) but I think something on the automated front is very much the way to go so we can keep track of where we stand.

I don't think we should import any python code into this repo though. Maybe we make a fork of the bitcoin tests in a separate repo? Not sure. Need to think on this issue a bit.

Also, as you point out, a bunch of this is for wallet and I honestly haven't given any thought to that repo yet.

@jakesylvestre
Copy link
Collaborator Author

I'm also going to need to create one of these around BIP's, so far I'm seeing (e.g. #1208). This isn't as straight forward as what I've done above but definitely something we need done to start prioritizing features

@jcvernaleo
Copy link
Member

Agreed, that one is a bit harder to do, but totally useful.

@Roasbeef
Copy link
Member

I haven't looked at the bitcoin python tests in a while (maybe a year or two) but I think something on the automated front is very much the way to go so we can keep track of where we stand. I don't think we should import any python code into this repo though. Maybe we make a fork of the bitcoin tests in a separate repo? Not sure. Need to think on this issue a bit.

We already have a test of RPC level integration tests: https://github.com/btcsuite/btcd/tree/master/integration

@Roasbeef
Copy link
Member

As far as the RPCs, my personal stance is that we no longer need to attempt to mirror each RPC change bitcoind makes. Their RPC interface has changed significantly over the past few years, and contains a number of wallet specific calls that don't apply to btcd at all since it has no default wallet. Instead IMO, we should make the btcjson+client package into a sub-module so it can be worked on (for those wanting to use it as an RPC client against bitcoind), independent of if we support that call on the server-side or not. We'd then do new release for those two packages (new module versions) as we started to update the calls.

@jakesylvestre
Copy link
Collaborator Author

I like this approach - it also allows us to focus on compatibility around BIPS

@onyb
Copy link
Collaborator

onyb commented May 16, 2020

@jakesyl Is it possible to move the feature-parity matrix to the Wiki, so we can collaboratively keep it updated?

@jakesylvestre
Copy link
Collaborator Author

@onyb absolutely

@jakesylvestre
Copy link
Collaborator Author

@federicobond
Copy link
Contributor

I would like to help speed up Bitcoin Core compatibility in the rpcclient module, as we are beginning to use it more extensively in our lnd+bitcoind systems at Muun (and apparently other Go users too judging by the number of stale pull requests).

@Roasbeef above suggested moving the btcjson+rpcclient modules out of the btcd repo. I think this is a sensible approach and I offer help with bug/feature triage and ongoing maintenance if someone from the @btcsuite organization kickstarts this project by creating a new repo for the rpcclient and moving the code there.

@federicobond
Copy link
Contributor

Hi! Any update on this? I can help out with the migration if necessary.

@jakesylvestre
Copy link
Collaborator Author

@federicobond want to hop in irc?

@federicobond
Copy link
Contributor

@jakesyl where?

@onyb
Copy link
Collaborator

onyb commented Aug 27, 2020

@federicobond #btcd on the Freenode network.

@elliottminns
Copy link
Contributor

Is the getblocktemplate RPC implemented on the client side? When I search the code I come across this:

// TODO(davec): Implement GetBlockTemplate

@onyb
Copy link
Collaborator

onyb commented Sep 8, 2020

@elliottminns We have the btcjson structs in place, but not supported in rpcclient yet.

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

No branches or pull requests

6 participants