AustEcon / bitsv

BitSV: Bitcoin made easy. Documentation:
https://AustEcon.github.io/bitsv
MIT License
96 stars 28 forks source link

inability to handle pre-generated 'false return' metadata from other libraries #52

Open AustEcon opened 4 years ago

AustEcon commented 4 years ago

The my_key.send([], message=data, custom_pushdata=True) works by allowing the user to pre-generate metadata with the functions contained within op_return.py (which generates a bytearray with the correct op_pushdata op codes for as many elements as there are). But it is the send() that adds the OP_RETURN to the beginning of these bytes. This has caused some users issues when they pregenerate these bytes using other libraries (which have already added the OP_RETURN.

They have gotten it to work by simply removing this first OP_RETURN byte to let bitsv add it but this is a hack and I think it is great to see people using bitsv in combination with other libraries and would like to support it.

I wonder if we cannot simply make the op_return.py functions for generating the bytes fall into line with how these other libraries are doing things.

In hindsight, it actually makes more sense to do it this way so that people can pre-generate any kind of crazy, complicated locking scripts and append them into a transaction as they see fit.

I don't know the best way to implement this though - and haven't given it much thought...

We could keep the message and custom_pushdata kwargs for backwards compatibility but deprecate these after a few releases maybe?

Open to suggestions...

I think I can make the send_op_return() work nicely without anyone even noticing that it's changed under the hood... but the send() is like the "low level" function that does everything if you really need it to and so would be nice to have it as intuitive as possible...

ghost commented 4 years ago

Hmm. So should we be expecting raw bytes or opcodes?

I think maybe could have opcodes= and then you add opcodes, data, whatever, and it's good to go. Andcustom_pushdata that adds opcodes to prefix the data.

AustEcon commented 4 years ago

I don't want it to be too confusing.

What if we have it something like: my_key.send(outputs=[], custom_outputs=[bytes_output1, bytes_output2, bytes_output3])

And provide the tools (update op_return.py functions) to generate full outputs rather than full outputs minus the op_return pre-fix!.

And we could still keep the "old way" of message=encoded_bytes and custom_pushdata=False/True But update documentation to encourage people to use the top method because it handles any situation going forward... E.g. a "Hello World" example for send() would be:

1 - Generate list of full outputs as a list of encoded bytes: ( requires new functions added to op_return.py file that could expand into their own 3rd party libraries over time...) custom_outputs = bitsv.op_return.create_false_return_output( ["Hello World".encode('utf-8')] )

later can have a suite of custom TxBuilder functions like bitsv.op_return.create_xyz_output() e.g. create_p2pkh_output(), create_r_puzzle_output() (shrug maybe or in separate repo).

2 - Add these full outputs to a transaction (correct fee etc. is calculated for you by default) my_key.send(outputs=[], custom_outputs=custom_outputs, fee=1)

This ^^ way requires no change to existing behavior but should be quite future proof for after Genesis upgrade with OP_RETURN restored to original functionality.

Fee calculation might need a bit of work but by providing a new kwarg as custom_outputs... we can handle this special case with care... i.e. check if there is some output amount being sent and factor that into the totals...