Try both TX serialization formats in createrawtransaction #1502
Conversation
|
LGTM, but maybe leave a comment for context? Just a short version of what you commented here would be nice, for example |
|
Tbh, @bjornoj I think the existing comment is clear enough, not sure we need anything more verbose |
| return nil, err | ||
| // we try both the new and old encoding format | ||
| witnessErr := msgTx.Deserialize(bytes.NewReader(serializedTx)) | ||
| if witnessErr != nil { | ||
| legacyErr := msgTx.DeserializeNoWitness(bytes.NewReader(serializedTx)) | ||
| if legacyErr != nil { | ||
| return nil, legacyErr | ||
| } |
There was a problem hiding this comment.
If both return an error, would the call to msgTx.Deserialize(...) return something useful that isn't already covered by msgTx.DeserializeNoWitness(...)?
There was a problem hiding this comment.
Sorry, I'm not sure I understand what you mean. Is this about which error to return?
There was a problem hiding this comment.
Sort of, yeah. Although I also just looked at the code and it's fine to do it this way. Let's say both msgTx.Deserialize and msgTx.DeserializeNoWitness both return an error. In this case, we go down the if witnessErr != nil branch and eventually return (nil, legacyErr). But there is a witnessErr which is non-nil and I was wondering if it might make sense to return both error messages.
I just checked the code though and both just make calls to msg.BtcDecode so it should be fine.
|
@jcvernaleo (as per #1530)
This has been pretty closely reviewed/run locally by myself and @Rjected so probably mergable |
torkelrogstad
force-pushed
the
2019-12-02-createrawtransaction-receive-btcd
branch
from
632b815
to
302717b
Compare
Currently, it is not possible to use the
rpcclientpackage to create a raw TX with Bitcoin Core if the transaction you're creating has no inputs. This is a realistic scenario, as you would typically callcreaterawtransactionwith your outputs specified, and then follow up withfundrawtransaction.The reason the RPC call fails is that we assume the returned transaction is encoded with the new witness format that was introduced with Segwit. We solve this by simply trying both serialization formats. If its possible to detect which format is used (so we avoid doing it twice) I'll update the PR.