Skip to content
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

backport: bitcoin#16333, #21862, #22385, #22550, #22597, #22632, #22718, #22907 - fire up test chains by first block - 2/n #6189

Merged
merged 9 commits into from
Aug 14, 2024

Commits on Aug 9, 2024

  1. Merge bitcoin#22385: refactor: Use DeploymentEnabled to hide VB deplo…

    …yments
    
    fa5658e Use DeploymentEnabled to hide VB deployments (MarcoFalke)
    fa11fec doc: Move buried deployment doc to the enum that enumerates them (MarcoFalke)
    
    Pull request description:
    
      Plus a doc commit.
    
    ACKs for top commit:
      jnewbery:
        utACK fa5658e
      ajtowns:
        utACK fa5658e
    
    Tree-SHA512: 2aeceee0674feb603d76656eff40695b7d7305de309f837bbb6a8c1dbb1d0b962b741f06ab7b9a8b1dbd1964c9c0c9aa5dc9588fd8e6d896e620b69e08eedbaa
    MarcoFalke authored and knst committed Aug 9, 2024
    Configuration menu
    Copy the full SHA
    fb00431 View commit details
    Browse the repository at this point in the history
  2. Merge bitcoin#22550: test: improve test_signing_with_{csv,cltv} sub…

    …tests (speed, prevent timeout)
    
    12f094e test: use constants for CSV/CLTV activation heights in rpc_signrawtransaction (Sebastian Falbesoner)
    746f203 test: introduce `generate_to_height` helper, use in rpc_signrawtransaction (Sebastian Falbesoner)
    e3237b1 test: check that CSV/CLTV are active in rpc_signrawtransaction (Sebastian Falbesoner)
    
    Pull request description:
    
      This PR primarily aims to solve the current RPC timeout problem for test rpc_signrawtransaction.py, as described in bitcoin#22542. In the course of that the test is also improved in other ways (see bitcoin#22542 (review)).
    
      Reviewers guideline:
      * In `test_signing_with_cltv()`, a comment is fixed -- it wrongly referred to CSV, it should be CLTV.
      * As preparation, assertions are added that ensure that CSV and CLTV have been really activated after generating blocks by checking the 'softforks' output of the getblockchaininfo() RPC. Right now in master, one could remove (or decrease, like in bitcoin#22542) the generate calls and the test would still pass, when it shouldn't.
      * A helper `generate_to_height()` is introduced which improves the previous way of reaching a block height in two ways:
          - instead of blindly generating TH blocks to reach target block height >= TH, the current block height CH is taken into account, and only (TH - CH) are generated in total
          - to avoid potential RPC timeouts, the block generation is split up into multiple generatetoaddress RPC calls ([as suggested by laanwj](bitcoin#22542 (comment))); here chunks of 200 blocks have been chosen
       * The helper is used in the affected sub-tests, which should both speed-up the test (from ~18s to ~12s on my machine) and avoid potential timeouts
       * Finally, the activation constants for CSV and CLTV are used instead of using magic numbers 500 and 1500
    
      Open questions:
      * Any good naming alternatives for `generate_to_height()`? Not really happy with the name, happy to hear suggestions
      * Where to put the CSV and CLTV activation height constants in the test_framewor folder? I guess importing constants from other tests isn't really the desired way to go
    
    ACKs for top commit:
      laanwj:
        Code review and tested ACK 12f094e
      rajarshimaitra:
        reACK bitcoin@12f094e
    
    Tree-SHA512: 14509f6d3e5a5a05d6a30a3145bb82cd96a29d9d8a589abf1944a8bf34291cde78ce711195f52e9426588dc822b3618ec9b455e057360021ae46152bb7613516
    MarcoFalke authored and knst committed Aug 9, 2024
    Configuration menu
    Copy the full SHA
    8928146 View commit details
    Browse the repository at this point in the history
  3. Merge bitcoin#22597: consensus/params: simplify ValidDeployment check…

    … to avoid gcc warning
    
    0591710 consensus/params: simplify ValidDeployment check to avoid gcc warning (Anthony Towns)
    
    Pull request description:
    
      Simplifies the ValidDeployment check to only check the upperbound, relying on the lower bound to be trivially true due to enum values starting at the minimum value of the underlying type (which is checked at compile time in deploymentstatus.cpp). Avoids a "comparison always true" warning in some versions of gcc.
    
      Fixes bitcoin#22587
    
    ACKs for top commit:
      MarcoFalke:
        review ACK 0591710
      tryphe:
        retested ACK 0591710
    
    Tree-SHA512: e53b5d478b46d35ec476d004e3c92803afb874c138dd6ef3848861f4281cc113fe88921bc4ac74fd4decbf318ed776d3f816c3a1185f99dc36a5cfecfff51f7c
    MarcoFalke authored and knst committed Aug 9, 2024
    Configuration menu
    Copy the full SHA
    cbd2be8 View commit details
    Browse the repository at this point in the history

Commits on Aug 12, 2024

  1. Merge bitcoin#22632: test: Set regtest.BIP66Height = 102 to speed up …

    …tests
    
    fafe896 test: Set regtest.BIP66Height = 102 to speed up tests (MarcoFalke)
    
    Pull request description:
    
      No need to waste time by forcing creation of more than 1000 blocks to get the benefits of being able to test BIP 66. Also, reducing the height makes it more likely that (third-party) tests are conforming to BIP 66, which is enforced on mainnet for all new blocks.
    
    ACKs for top commit:
      GeneFerneau:
        Concept + code review ACK [fafe896](bitcoin@fafe896)
      0xB10C:
        crACK fafe896
      laanwj:
        ACK fafe896
      Zero-1729:
        tACK fafe896
      kristapsk:
        ACK fafe896. Full functional test suite showed few second speed incrase on my laptop (although I didn't do proper benchmarking with multiple runs, just single `time ./test/functional/test_runner.py` on current master vs this PR).
      theStack:
        Tested ACK fafe896
      hg333:
        tACK bitcoin@fafe896
    
    Tree-SHA512: 4bbee3c8587d612e74a59fde49b6439c1296f2fc27d3a7cf59a35e920f729fdd581c930290bd04def618f81412236676ddb99b4ceb4d80dfb9fd610b128a04b1
    laanwj authored and knst committed Aug 12, 2024
    Configuration menu
    Copy the full SHA
    fc25503 View commit details
    Browse the repository at this point in the history
  2. Merge bitcoin#22907: test: Avoid intermittent test failure in feature…

    …_csv_activation.py
    
    fa676db test: pep-8 whitespace (MarcoFalke)
    faed284 test: Avoid intermittent test failure in feature_csv_activation.py (MarcoFalke)
    
    Pull request description:
    
      Otherwise there will be disconnects if the test runs longer than the default peertimeout (60s):
    
      ```
       node0 2021-09-05T20:28:30.973116Z (mocktime: 2021-09-01T07:17:29Z) [net] [net.cpp:1323] [InactivityCheck] socket receive timeout: 393061s peer=0
      ```
    
      Fix that by skipping `InactivityCheck` via a large `-peertimeout`.
    
    ACKs for top commit:
      fanquake:
        ACK fa676db
    
    Tree-SHA512: 061c0585a805aa2f8e55c4beedd4b8498a2951f33d60aa3632dda0a284db3a627d14a23dbd57e8a66c69a1612f39418e3a755c8ca97f6ae1105c0d70f0d1a801
    merge-script authored and knst committed Aug 12, 2024
    Configuration menu
    Copy the full SHA
    71af881 View commit details
    Browse the repository at this point in the history
  3. Merge bitcoin#16333: test: Set BIP34Height = 2 for regtest

    222290f test: Set BIP34Height = 2 for regtest (MarcoFalke)
    fac90c5 test: Create all blocks with version 4 or higher (MarcoFalke)
    
    Pull request description:
    
      BIP34 is active on the current tip of mainnet, so all miners must obey it. It would be nice if it also was active in fresh regtest instances from the earliest time possible.
    
      I changed the BIP34 height to `2`, so that the block at height=1 may be used to mine a duplicate coinbase. (Needed to test mainnet behaviour)
    
      This pull is done in two commits:
    
      *  test: Create all blocks with version 4 or higher:
         Now that BIP34 is activated earlier, we need to create blocks with a higher version number. Just bump it to 4 instead of 2 to avoid having to bump it again later.
    
      *  test: Set BIP34Height = 2 for regtest:
         This fixes the BIP34 implementation in the tests (to match the one of the Core codebase) and updates the tests where needed
    
    ACKs for top commit:
      ajtowns:
        ACK 222290f
      jonatack:
        ACK 222290f tested and reviewed rebased to current master 5e21382
      theStack:
        Tested ACK 222290f
    
    Tree-SHA512: d69c637a62a64b8e87de8c7f0b305823d8f4d115c1852514b923625dbbcf9a4854b5bb3771ff41702ebf47c4c182a4442c6d7c0b9f282c95a34b83e56a73939b
    MarcoFalke authored and knst committed Aug 12, 2024
    Configuration menu
    Copy the full SHA
    101a863 View commit details
    Browse the repository at this point in the history
  4. Merge bitcoin#22718: doc: Add missing PR 16333 release note

    fa76ebd doc: Add missing PR 16333 release note (MarcoFalke)
    
    Pull request description:
    
      Missed in commit ad0fc45
    
    ACKs for top commit:
      laanwj:
        Documentation review ACK fa76ebd
      Zero-1729:
        ACK fa76ebd
      theStack:
        ACK fa76ebd
    
    Tree-SHA512: 5fc9e3f61d2f8534f537cc14c7518d09babd5ebbedd5f24ad10ca9aff69d499f3daa0c45eb548eec94cdbf25bf53c9f0158ec2ee7aa5c3f653f7670c68d1d24c
    MarcoFalke authored and knst committed Aug 12, 2024
    Configuration menu
    Copy the full SHA
    adcf095 View commit details
    Browse the repository at this point in the history
  5. Merge bitcoin#21862: test: Set regtest.BIP65Height = 111 to speed up …

    …tests
    
    faf7e48 Set regtest.BIP65Height = 111 to speed up tests (MarcoFalke)
    
    Pull request description:
    
      No need to waste time by forcing creation of more than 1000 blocks to get the benefits of being able to test BIP 65. Also, reducing the height makes it more likely that (third-party) tests are conforming to BIP 65, which is enforced on mainnet for all new blocks.
    
    ACKs for top commit:
      theStack:
        re-ACK faf7e48 📍
      Zero-1729:
        re-ACK faf7e48
      kristapsk:
        ACK faf7e48
    
    Tree-SHA512: 79a8263e7233838666b9b636b496a8b9eb12398c779f9434677e1d62816732c0a7c7b3e73965be1fb0038d35e05e5a90e665bd74e9610104127dfc4ea38169bf
    fanquake authored and knst committed Aug 12, 2024
    Configuration menu
    Copy the full SHA
    65b92fa View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    e4e7c44 View commit details
    Browse the repository at this point in the history