Add witness option to Transaction toBuffer()

[benjamin-wilson]

During construction of a coinbase transaction for mining I have to get the serialized coinbase transaction to send to the miners. The miners do not expect the witness data to be in the header. Currently I resort to using the private __toBuffer() method. coinbaseTransaction.__toBuffer().toString('hex'); It would be nice if the toBuffer() and toHex() methods had an option to exclude the witness data.

[jasonandjay]

@benjamin-wilson @ljluestc
Which scenario requires excluding witness data?

[benjamin-wilson]

This is a step in the construction of coinbase transactions for mining

[JacksonDMiller]

Hi @benjamin-wilson,

I looked into adding this for you, but I think you might be working with outdated code or assumptions. Since Segwit was introduced, coinbase transactions have included a witness reserved value for compatibility with Segwit. While it is still possible to construct a coinbase transaction in the way you want, the block would not be able to include any Segwit transactions, so it would be a very inefficient way to mine.

You can read more about coinbase transactions here.

If you have more questions I would be happy to help or if this answers your question than you can close this issue.

[benjamin-wilson]

It's been awhile since I've looked at this but it's not the reserved value that's the problem (I use it) but the other witnesses data. You can see how I use it here:
https://github.com/benjamin-wilson/public-pool/blob/8ce057b3b51c33bf915f57f3a30822b449613276/src/models/MiningJob.ts#L79

You can also check out the createCoinbaseTransaction function to see how I do the construction

[junderw]

witness stripping seems like a pretty niche use case...

if we were to expose it to the public API, it might make more sense to offer a stripWitnesses method that loops through all ins and replaces witness with EMPTY_WITNESS

But no, the answer is not a parameter to toBuffer.

If there is a witness on one of the inputs we must encode it as witness encoding. Adding the stripping logic to the encoding method is more error prone.

[benjamin-wilson]

IMO making functions for mining Bitcoin in bitcoinjs isn't niche.
Parameterizing to buffer() was just a suggestion, whichever way it's decided best to architect is good with me. Why was this issue closed as completed?

[junderw]

Why was this issue closed as completed?

Because that's the default, and this repository does not make use of the new close features.

When the issue is closed it's closed.