bitsv icon indicating copy to clipboard operation
bitsv copied to clipboard

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

Open AustEcon opened this issue 4 years ago • 2 comments

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...

  • I wonder if "custom_pushdata" could be changed to something more like "custom_output=True" and maybe change message --> custom_output(s) (accepting a list of them would future-proof us for a future with multiple custom outputs) ...

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...

AustEcon avatar Sep 29 '19 10:09 AustEcon

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.

ghost avatar Sep 29 '19 18:09 ghost

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...

AustEcon avatar Sep 29 '19 23:09 AustEcon