Just something I noted when using these externally. The new ordering matches how they are encoded in the serialised transaction.
If there was a reason for these to be ordered this way, then I've probably misinterpreted the test data.
Just something I noted when using these externally. The new ordering matches how they are encoded in the serialised transaction.
If there was a reason for these to be ordered this way, then I've probably misinterpreted the test data.
If there was a reason for these to be ordered this way, then I've probably misinterpreted the test data.
That depends: is the re-ordering part of the test or not?
That depends: is the re-ordering part of the test or not?
The tests don't fail with this new ordering, and the resultant transaction is encoded the same. There does not appear to be any hints in the associated comments that the ordering was important either.
if (mapprevOutValues.count(tx.vin[i].prevout)) {
In the tests, they are just using a map-lookup to see if the (transaction decoded) inputs were in the provided list. Ultimately it is the encoded transaction that was dictating the order (as can be seen in the "fixed" data).
No strong opinion about changing the order, but I think it makes sense to be consistent, if there was no particular reason for the current order.
@NicolasDorier Was there a particular reason you chose that order?
None, no strong opinion either. I generated the tests from some C# code.
The motivation for this change was that it was @un-intuitive at first why my tests were failing in an external open-source library that was testing against these inputs 1:1 (that is, I had an underlying assumption of the inputs being in-order).
Putting the test fixtures in order also reduces the potential complexity of a test by allowing the inputs to be matched with a simple for loop rather than doing a double-iteration or map-lookup.
Anything further needed here?