- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.8k
BIP54: Consensus Cleanup test vectors #2015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Open
      
      
            darosior
  wants to merge
  3
  commits into
  bitcoin:master
  
    
      
        
          
  
    
      Choose a base branch
      
     
    
      
        
      
      
        
          
          
        
        
          
            
              
              
              
  
           
        
        
          
            
              
              
           
        
       
     
  
        
          
            
          
            
          
        
       
    
      
from
darosior:2509_consensus_cleanup_test_vectors
  
      
      
   
  
    
  
  
  
 
  
      
    base: master
Could not load branches
            
              
  
    Branch not found: {{ refName }}
  
            
                
      Loading
              
            Could not load tags
            
            
              Nothing to show
            
              
  
            
                
      Loading
              
            Are you sure you want to change the base?
            Some commits from the old base branch may be removed from the timeline,
            and old review comments may become outdated.
          
          
      
        
          +26,112
        
        
          −1
        
        
          
        
      
    
  
  
     Open
                    Changes from all commits
      Commits
    
    
  File filter
Filter by extension
Conversations
          Failed to load comments.   
        
        
          
      Loading
        
  Jump to
        
          Jump to file
        
      
      
          Failed to load files.   
        
        
          
      Loading
        
  Diff view
Diff view
There are no files selected for viewing
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              | Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| ## BIP54 test vectors | ||
|  | ||
| This folder contains a set of test vectors for each mitigation introduced in the BIP. This document | ||
| presents them in more detail. | ||
|  | ||
| The code used to generate half of the test vectors is included with the implementation and available | ||
| [here][other-vectors]. The other half requires mining mainnet blocks and is [published | ||
| separately][bip54-miner]. In both cases it is implemented as a regular Bitcoin Core unit test, and | ||
| the test vectors are persisted as a JSON file if the `UPDATE_JSON_TESTS` preprocessor directive is | ||
| set (off by default). | ||
|  | ||
| To compile the [header][header-miner] and [block][block-miner] miners you may have to link to | ||
| libatomic explicitly. This can be achieved like so: | ||
| ``` | ||
| cmake -B atomicbuild -DAPPEND_LDFLAGS="-latomic" | ||
| cmake --build atomicbuild/ -j $(nproc) | ||
| ``` | ||
|  | ||
| [Premined headers][premined-headers] are also provided along with the header miner to allow changing | ||
| some of the last headers without having to re-generate the whole chain. | ||
|  | ||
|  | ||
| ### Difficulty adjustment exploits | ||
|  | ||
| The [`timestamps.json`](./timestamps.json) test vectors exercise the two constraints on block header | ||
| timestamps introduced by BIP54 to mitigate the Timewarp and Murch-Zawy attacks. Each test case | ||
| features a chain of mainnet headers starting from the genesis block, and whether this header chain | ||
| is valid by BIP54 rules. Each test case also contains a comment describing why this particular chain | ||
| is (in)valid according to BIP54. All test cases are valid according to current Bitcoin consensus | ||
| rules. It is intended to be used to test a BIP54 implementation by feeding the header chain to a | ||
| Bitcoin node implementation, enforcing the BIP54 rules on this chain from genesis. | ||
|  | ||
| The test vector file features a JSON array of JSON objects, each corresponding to a test case. Each | ||
| JSON object features the following entries: | ||
| - `header_chain`: a JSON array of strings. An ordered list of hex-encoded mainnet block headers. | ||
| - `valid`: a JSON boolean. Whether this chain of headers is valid according to BIP54. | ||
| - `comment`: a JSON string. Description of the test case. | ||
|  | ||
| For the purpose of testing a Timewarp fix, a Timewarp attack was included early on in the history of | ||
| testnet3. An implementer of BIP54 may want to ensure that syncing testnet3 by enforcing BIP54 since | ||
| genesis will treat block `00000000118da1e2165a19307b86f87eba814845e8a0f99734dce279ca3fb029` as | ||
| invalid. | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very open question: how hard it would be to coordinate a Murch-Zawy attack variant on testnet3. | ||
|  | ||
|  | ||
| ### Long block validation time | ||
|  | ||
| The [`sigops.json`](sigops.json) file contains test vectors for the limit on the number of | ||
| potentially-executed legacy signature operations in a single transaction, introduced by BIP54 in | ||
| order to mitigate long block validation times. Each test case represents a transaction and whether a | ||
| block containing it would be valid according to BIP54. The test cases feature an extensive set of | ||
| combinations of inputs and output types, ways to run into the limit, historical violations and some | ||
| pathological transactions exhibiting the specific implementation details. All test cases but those | ||
| belonging to this last category feature transactions that are valid under current Bitcoin consensus | ||
| rules. Each test case also features a comment describing why the transaction is (in)valid according | ||
| to BIP54. | ||
|  | ||
| The test vector file features a JSON array of JSON objects, each corresponding to a test case. Each | ||
| JSON object features the following entries: | ||
| - `spent_outputs`: a JSON array of strings. An ordered list of hex-encoded Bitcoin-serialized | ||
| transaction outputs spent by each input of this test case's transaction. | ||
| - `tx`: a JSON string. A hex-encoded Bitcoin-serialized transaction to be evaluated. | ||
| - `valid`: a JSON boolean. Whether this transaction is valid according to current consensus rules | ||
| supplemented by BIP54. | ||
| - `comment`: a JSON string. Description of the test case. | ||
|  | ||
|  | ||
| ### Merkle tree malleability with 64-byte transactions | ||
|  | ||
| The [`txsize.json`](./txsize.json) file contains test cases exercising the new constraint on | ||
| non-witness transaction size introduced in BIP54. Each test case contains a transaction and whether | ||
| it would be valid according to BIP54, as well as a comment describing why it is (in)valid. All test | ||
| cases are otherwise valid according to current Bitcoin consensus rules. | ||
|  | ||
| The test vector file features a JSON array of JSON objects, each corresponding to a test case. Each | ||
| JSON object features the following entries: | ||
| - `tx`: a JSON string. A hex-encoded Bitcoin-serialized transaction to be evaluated. | ||
| - `valid`: a JSON boolean. Whether this transaction is valid according to BIP54. | ||
| - `comment`: a JSON string. Description of the test case. | ||
|  | ||
|  | ||
| ### Possibility of duplicate coinbase transactions | ||
|  | ||
| The [`coinbases.json`](./coinbases.json) file contains test cases exercising the new restrictions on | ||
| coinbase transactions introduced in BIP54 to prevent duplicate coinbase transactions without | ||
| resorting to BIP30 validation. Each test case contains a chain of mainnet blocks (including the | ||
| genesis block), and whether this block chain is valid according to BIP54. All test cases are valid | ||
| according to current Bitcoin's consensus rules, except one that features a block containing a | ||
| coinbase transaction timelocked to a future block height. | ||
|         
                  darosior marked this conversation as resolved.
              Show resolved
            Hide resolved | ||
|  | ||
| The test vector file features a JSON array of JSON objects, each corresponding to a test case. Each | ||
| JSON object features the following entries: | ||
| - `block_chain`: a JSON array of strings. An ordered list of hex-encoded mainnet blocks. | ||
| - `valid`: a JSON boolean. Whether this block chain is valid according to current Bitcoin consensus | ||
| rules supplemented by BIP54. | ||
| - `comment`: a JSON string. Description of the test case. | ||
|  | ||
|  | ||
| [bip54-miner]: https://github.com/darosior/bitcoin/blob/bip54_miner/commits | ||
| [header-miner]: https://github.com/darosior/bitcoin/blob/bip54_miner/src/test/bip54_header_miner.cpp | ||
| [block-miner]: https://github.com/darosior/bitcoin/blob/bip54_miner/src/test/bip54_block_miner.cpp | ||
| [other-vectors]: https://github.com/darosior/bitcoin/blob/2509_inquisition_consensus_cleanup/src/test/bip54_tests.cpp | ||
| [premined-headers]: https://github.com/darosior/bitcoin/blob/bip54_miner/src/test/bip54_premined_headers.h | ||
      
      Oops, something went wrong.
        
    
  
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Started to look on the
timestamps.jsonfile, I’m understanding that the headers for the 9 chain of headers have been drawn the historical mainnet chain, and then permutation of a subset of those chain of headers have been made accordingly to make some of them (in)valid according to BIP54, assuming that BIP54 would have been enforced as a rule since the genesis block.Given the difficulty adjustment is proposing rules for the validation state where block is equal to height
N % 2016equal 0 and the state where block is equal to heightN % 2016equal 2016, and there is at least >, =, < to test for each boundary check, there should be at least 6 tests. From checkingtimestamps.json, there at least 9 test chains in the file.Wondering if there is full coverage for the first equality check, given you have > and = to test each for the variations of minus 7200 (and then same, you have >, =, < I think that are valuable to test for).