search

AustEcon / bitsv

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

fix op_return output order: bitsv places last, bico.media assumes first #48

Closed xloem closed 4 years ago

xloem commented 5 years ago

None of the polyglot uploads are visible on bico.media anymore (https://bico.media/be8b6a79e66934d3419265fbf3295d03e331a4c08098ae7f817a7592ffaedd2b gives "Not found..."), and it seems to be because bitsv doesn't place the OP_RETURN as the first output. Instead it places it last. The relevent line to fix is https://github.com/AustEcon/bitsv/blob/5a2c4b7c5dd9470d99d05c2ff7aac33ed81b5666/bitsv/transaction.py#L211 , I guess in such a way that messages is placed at the start of the outputs rather than the end.

I mentioned the issue in unwriter's slack channel, and another dev said they also assumed the op_return was first in their code, so it seems good practice to put it there. This is of course also a bug in bico.media, which I haven't reported at this time. Not sure how.

AustEcon commented 5 years ago

Okay.

Thank you for investigating this. Will fix it as soon as I can but may not be until weekend or later.

PS: in theory is a really simple fix though...

AustEcon commented 5 years ago

I can confirm that putting OP_RETURN as first output fixes the issue with rendering on bico.media. https://github.com/AustEcon/bitsv/commit/40003ee8ebef8db996d0f9b6210cf696b3324d7e

ghost commented 5 years ago

Nice fix!

Is this something that you can add a test case for without too much trouble?

AustEcon commented 5 years ago

Yep! I also broke about 6 tests because of the change in order... ran out of time last night.

ghost commented 5 years ago

Ok, no problem! :)

It happens.