Add signrawtransactionwithwallet command interface #1561
Conversation
This change makes btcd's getblock command match bitcoind's. Previously the default verbosity was 0, which caused errors when using the rpcclient library to connect to a bitcoind node - getblock would unmarshall incorrectly since it didn't expect a verbosity=1 result when it did not specify verbosity.
Adds interface for issuing a signrawtransactionwithwallet command. Note that this does not add functionality for the btcd rpc server itself, it simply assumes that the RPC client has this ability and gives an API for interacting with the RPC client.
|
Hey, any chance you could force push here so we can get this merged? Github has an issue when required tests are swapped |
|
Force push where? |
Pull Request Test Coverage Report for Build 276939909Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
|
I'm good to merge once we rebase into a single commit |
|
Sorry this is a workflow I'm not used to and I don't want to mess anything up. You want me to rebase everything onto the oldest commit here? Looks like the oldest commit on the PR is e82643f so given your request I would take its parent 57d44d0 and run Also can you not just squash and merge? |
btcjson/walletsvrcmds.go
Outdated
| type SignRawTransactionWithWalletCmd struct { | ||
| RawTx string | ||
| Inputs *[]RawTxWitnessInput | ||
| Flags *string `jsonrpcdefault:"\"ALL\""` |
There was a problem hiding this comment.
Let's use SigHashType as the field name here.
| // function on the returned instance. | ||
| // | ||
| // See SignRawTransactionWithWallet2 for the blocking version and more details. | ||
| func (c *Client) SignRawTransactionWithWallet2Async(tx *wire.MsgTx, |
There was a problem hiding this comment.
I see that coming up with the right function names is a bit tricky here. The issue with SignRawTransactionWithWallet# is that you need to read the documentation of all the variants to know which one to use.
I suggest using something more verbose, even if it makes the name inconveniently long. Something like:
SignRawTransactionWithWalletSignRawTransactionWithWalletUsingInputsSignRawTransactionWithWalletUsingInputsSighash
There was a problem hiding this comment.
Actually after seeing SignRawTransaction#, I'm kind of split. 🙂 So maybe it's better to follow the current convention, even if it's not so great.
|
@henryperson Rebase won't be easy in your case, due to some unnecessary commits in your branch. I suggest you clean up your branch in the following way: |
Adds interface for issuing a signrawtransactionwithwallet command. It's heavily based on the code for signrawtransaction, which has been deprecated as of Bitcoin Core v0.17.0. Note that this does not add functionality for the btcd RPC server itself, it simply assumes that the RPC client one is connected to has this command already and gives an API for issuing the command. In other words, this works with a bitcoind client but not a btcd client.