Implement canonical transaction ordering.

Summary:
This ensure that transaction are ordered by txid going forward.

Depends on D1528, D1527

Test Plan: Added integration tests.

Reviewers: #bitcoin_abc, schancel, jasonbcox

Reviewed By: #bitcoin_abc, schancel, jasonbcox

Subscribers: jasonbcox, schancel, teamcity

Differential Revision: https://reviews.bitcoinabc.org/D1529

src/miner.cpp

@@ -175,6 +175,16 @@ BlockAssembler::CreateNewBlock(const CScript &scriptPubKeyIn) {
     int nDescendantsUpdated = 0;
     addPackageTxs(nPackagesSelected, nDescendantsUpdated);
 
+    if (IsMagneticAnomalyEnabled(*config, pindexPrev)) {
+        // If magnetic anomaly is enabled, we make sure transaction are
+        // canonically ordered.
+        std::sort(std::begin(pblock->vtx) + 1, std::end(pblock->vtx),
+                  [](const std::shared_ptr<const CTransaction> &a,
+                     const std::shared_ptr<const CTransaction> &b) -> bool {
+                      return a->GetId() > b->GetId();
+                  });
+    }
+
     int64_t nTime1 = GetTimeMicros();
 
     nLastBlockTx = nBlockTx;

src/test/miner_tests.cpp

@@ -675,8 +675,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) {
     // Sequence locks fail.
     BOOST_CHECK(!TestSequenceLocks(CTransaction(tx), flags));
 
-    BOOST_CHECK(pblocktemplate =
-                    BlockAssembler(config).CreateNewBlock(scriptPubKey));
+    pblocktemplate = BlockAssembler(config).CreateNewBlock(scriptPubKey);
+    BOOST_CHECK(pblocktemplate);
 
     // None of the of the absolute height/time locked tx should have made it
     // into the template because we still check IsFinalTx in CreateNewBlock, but

src/validation.cpp

@@ -3684,9 +3684,28 @@ static bool ContextualCheckBlock(const Config &config, const CBlock &block,
                                         ? nMedianTimePast
                                         : block.GetBlockTime();
 
+    const bool fIsMagneticAnomalyEnabled =
+        IsMagneticAnomalyEnabled(config, pindexPrev);
+
     // Check that all transactions are finalized
-    for (const auto &tx : block.vtx) {
-        if (!ContextualCheckTransaction(config, *tx, state, nHeight,
+    const CTransaction *prevTx = nullptr;
+    for (const auto &ptx : block.vtx) {
+        const CTransaction &tx = *ptx;
+        if (fIsMagneticAnomalyEnabled) {
+            if (prevTx && (tx.GetId() < prevTx->GetId())) {
+                return state.DoS(
+                    100, false, REJECT_INVALID, "tx-ordering", false,
+                    strprintf("Transaction order is invalid (%s < %s)",
+                              tx.GetId().ToString(),
+                              prevTx->GetId().ToString()));
+            }
+
+            if (prevTx || !tx.IsCoinBase()) {
+                prevTx = &tx;
+            }
+        }
+
+        if (!ContextualCheckTransaction(config, tx, state, nHeight,
                                         nLockTimeCutoff)) {
             // state set by ContextualCheckTransaction.
             return false;

test/functional/abc-replay-protection.py

@@ -19,6 +19,7 @@
 
 # far into the future
 REPLAY_PROTECTION_START_TIME = 2000000000
+MAGNETIC_ANOMALY_START_TIME = 4000000000
 
 # Error due to invalid signature
 INVALID_SIGNATURE_ERROR = b'mandatory-script-verify-flag-failed (Signature must be zero for failed CHECK(MULTI)SIG operation)'
@@ -42,6 +43,7 @@ def set_test_params(self):
         self.tip = None
         self.blocks = {}
         self.extra_args = [['-whitelist=127.0.0.1',
+                            "-magneticanomalyactivationtime=%d" % MAGNETIC_ANOMALY_START_TIME,
                             "-replayprotectionactivationtime=%d" % REPLAY_PROTECTION_START_TIME]]
 
     def run_test(self):

test/functional/abc-transaction-ordering.py

@@ -220,33 +220,48 @@ def update_block(block_number, new_transactions=[]):
                      MAGNETIC_ANOMALY_START_TIME - 1)
 
         # Before we activate the Nov 15, 2018 HF, transaction order is respected.
-        def out_of_order_block(block_number, spend):
-            b = block(block_number, spend=spend, tx_count=3)
-            b.vtx[1], b.vtx[2] = b.vtx[2], b.vtx[1]
+        def ordered_block(block_number, spend):
+            b = block(block_number, spend=spend, tx_count=16)
+            b.vtx = [b.vtx[0]] + sorted(b.vtx[1:], key=lambda tx: tx.get_id())
             update_block(block_number)
             return b
 
-        out_of_order_block(4444, out[16])
+        ordered_block(4444, out[16])
         yield rejected(RejectResult(16, b'bad-txns-inputs-missingorspent'))
 
         # Rewind bad block.
         tip(5104)
 
         # Activate the Nov 15, 2018 HF
-        block(5556)
+        block(5556, out[16], tx_count=16)
         yield accepted()
 
         # Now MTP is exactly the fork time. Bigger blocks are now accepted.
         assert_equal(node.getblockheader(node.getbestblockhash())['mediantime'],
                      MAGNETIC_ANOMALY_START_TIME)
 
-        # Now that the fork activated, we can put transactions out of order in the block.
-        out_of_order_block(4445, out[16])
+        # Block with regular ordering are now rejected.
+        block(5557, out[17], tx_count=16)
+        yield rejected(RejectResult(16, b'tx-ordering'))
+
+        # Rewind bad block.
+        tip(5556)
+
+        # Now that the fork activated, we need to order transaction per txid.
+        ordered_block(4445, out[17])
         yield accepted()
 
-        oooblockhash = node.getbestblockhash()
-        node.invalidateblock(oooblockhash)
-        assert(node.getbestblockhash() != oooblockhash)
+        # Invalidate the best block and make sure we are back at the fork point.
+        ctorblockhash = node.getbestblockhash()
+        node.invalidateblock(ctorblockhash)
+        forkblockhash = node.getbestblockhash()
+        assert(forkblockhash != ctorblockhash)
+        assert_equal(node.getblockheader(forkblockhash)[
+                     'mediantime'], MAGNETIC_ANOMALY_START_TIME)
+
+        node.generate(1)
+        generatedblockhash = node.getbestblockhash()
+        assert(forkblockhash != generatedblockhash)
 
 
 if __name__ == '__main__':

test/functional/test_framework/mininode.py

@@ -422,6 +422,11 @@ def calc_sha256(self):
         self.hash = encode(
             hash256(self.serialize())[::-1], 'hex_codec').decode('ascii')
 
+    def get_id(self):
+        # For now, just forward the hash.
+        self.calc_sha256()
+        return self.hash
+
     def is_valid(self):
         self.calc_sha256()
         for tout in self.vout: