search

bitcoinjs / bitcoinjs-lib

A javascript Bitcoin library for node.js and browsers.
MIT License
5.7k stars 2.11k forks source link

Add witness option to Transaction toBuffer() #1951

Closed benjamin-wilson closed 1 month ago

benjamin-wilson commented 1 year ago

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 commented 6 months ago

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

benjamin-wilson commented 6 months ago

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

JacksonDMiller commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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.