test: Add test for BIP30 duplicate tx #16363

1. 
   MarcoFalke commented at 6:24 pm on July 9, 2019: member

   This adds a test for BIP30 to check that duplicate txs can exist in the blockchain given the first one was completely spent when the second one is added. (Requested by ajtowns in #16333 (comment))

   We can not add a test that a later duplicate tx overwrites a previous one, because BIP30 is always enforced on regtest. If someone feels strongly about such a test, some Bitcoin Core code would have to be modified, which can be done in a follow up pull request.

   Also, add a commit to fix the BIP34 test failures reported in #14633#issue-227712540

2. test: Properly serialize BIP34 coinbase height 77770d95e2
3. test: Add test for BIP30 duplicate tx fa8489a155
4. MarcoFalke force-pushed on Jul 9, 2019
5. fanquake added the label Tests on Jul 9, 2019
6. 
   DrahtBot commented at 11:07 pm on July 9, 2019: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Coverage

   Coverage	Change (pull 16363, c1f0f852484bf2470fae5d39d3b32231e7184f98)	Reference (master, 8046a3e0befeea641b6309bc0c742b7481e681d9)
   Lines	-0.0336 %	89.0827 %
   Functions	+0.0157 %	82.0163 %
   Branches	-0.0049 %	45.2266 %

   Updated at: 2019-07-09T23:07:22.556814.

7. 
   laanwj commented at 9:59 am on July 10, 2019: member

   Concept ACK
8. 
   sdaftuar commented at 1:27 pm on July 10, 2019: member

   We can not add a test that a later duplicate tx overwrites a previous one, because BIP30 is always enforced on regtest.

   I think we cannot add such a test because it would violate consensus rules, except for the two hardcoded exceptions on mainnet!

   Also I think it’s worth pointing out that the optimizations we have in place for mainnet, where we don’t actually enforce BIP30 explicitly if we’re on the BIP34-activating chain we expect to be on, makes regtest-testing less meaningful here. However, we do rely on BIP 30 being explicitly enforced for alternate chains, which is critical to allowing a node to reorg to the main chain if being fed an alternate early history, so this is probably a good test to add overall.

9. 
   MarcoFalke commented at 1:42 pm on July 10, 2019: member

   I think we cannot add such a test because it would violate consensus rules, except for the two hardcoded exceptions on mainnet!

   Yes, we’d have to add a regtest exception as well, and I’d prefer not to modify source code in a pull that only adds tests.

10. 
    ajtowns commented at 11:18 am on July 16, 2019: member

    We check that the hardcoded exceptions work correctly everytime someone does an IBD and we check the non-exceptional case every other block, so doesn’t seem like there’s much value in adding regtest-only exceptions to me…
11. laanwj added this to the "Blockers" column in a project

    ---

12. 
    laanwj commented at 11:02 am on August 1, 2019: member

    Code review ACK fa8489a15511f61a372473927e73c34692bbec23
13. MarcoFalke referenced this in commit 62117f9f36 on Aug 5, 2019
14. MarcoFalke merged this on Aug 5, 2019
15. MarcoFalke closed this on Aug 5, 2019

    ---

16. MarcoFalke deleted the branch on Aug 5, 2019
17. fanquake removed this from the "Blockers" column in a project

    ---

18. sidhujag referenced this in commit f44d5064d6 on Aug 10, 2019
19. jasonbcox referenced this in commit 8a6204742c on Oct 13, 2020
20. jasonbcox referenced this in commit 2e27aa6372 on Oct 13, 2020
21. Munkybooty referenced this in commit 2c0929fa2f on Nov 25, 2021
22. Munkybooty referenced this in commit 0b1215a5fc on Nov 25, 2021
23. Munkybooty referenced this in commit d0f1663305 on Nov 30, 2021
24. DrahtBot locked this on Dec 16, 2021

Contributors
MarcoFalke DrahtBot laanwj sdaftuar ajtowns

Labels
Tests