Transactions with more than one datacarrier (OP_RETURN) output
are never considered standard, i.e. this change is necessary in
order to to get rid of the `acceptnonstdtxn` option for some
tests.
Confirmed UTXOs in functional tests can simply be created by using
MiniWallet's `send_self_transfer_multi` method with a subsequent
`generate` call to mine a block.
b167e536d0 test: refactor: use `create_lots_of_big_transactions` to dedup where possible (Sebastian Falbesoner)
8973eeb412 test: use MiniWallet for mining_prioritisetransaction.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (mining_prioritisetransaction.py) to be run even with the Bitcoin Code wallet by using the MiniWallet instead, as proposed in #20078. Note that the adapted helper function `create_lots_of_big_transactions` is currently only used in this test, i.e. there was no need to change any others.
ACKs for top commit:
ayush933:
tACK b167e53
danielabrozzoni:
tACK b167e536d0
kouloumos:
ACK b167e536d0
furszy:
ACK b167e536
Tree-SHA512: ccae20d7d414a720efdeea9c2ae399aa53a3a0e7db72bff8d0cb75d90621a7ae7c019ba68d24f9d06f7b111f87ff33bb9d8e5aa08b763e606cf10268780e205c
a380922891 Release notes for getdeploymentinfo rpc (Anthony Towns)
240cad09ba rpc: getdeploymentinfo: include signalling info (Anthony Towns)
376c0c6dae rpc: getdeploymentinfo: include block hash/height (Anthony Towns)
a7469bcd35 rpc: getdeploymentinfo: change stats to always refer to current period (Anthony Towns)
7f15c1841b rpc: getdeploymentinfo: allow specifying a blockhash other than tip (Anthony Towns)
fd826130a0 rpc: move softfork info from getblockchaininfo to getdeploymentinfo (Anthony Towns)
Pull request description:
The aim of this PR is to improve the ability to monitor soft fork status. It first moves the softfork section from getblockchaininfo into a new RPC named getdeploymentinfo, which is then also able to query the status of forks at an arbitrary block rather than only at the tip. In addition, bip9 status is changed to indicate the status of the given block, rather than just for the next block, and an additional field is included to indicate whether each block in the signalling period signaled.
ACKs for top commit:
laanwj:
Code review and lightly tested ACK a380922891
Sjors:
tACK a380922891
fjahr:
tACK a380922891
Tree-SHA512: 7417d733b47629f229c5128586569909250481a3e94356c52fe67a03fd42cd81745246e384b98c4115fb61587714c879e4bc3e5f5c74407d9f8f6773472a33cb
fadc0c80ae p2p: Make timeout mockable and type safe, speed up test (MarcoFalke)
fa6d5a238d scripted-diff: Rename m_last_send and m_last_recv (MarcoFalke)
Pull request description:
Use type-safe time for better code readability/maintainability and mockable time for better testability. This speeds up the p2p_timeout test.
This is also a bugfix for intermittent test issues like: https://cirrus-ci.com/task/4769904156999680?command=ci#L2836Fixes#20654
ACKs for top commit:
laanwj:
Code review ACK fadc0c80ae
naumenkogs:
ACK fadc0c80ae
Tree-SHA512: 28c6544c97f188c8a0fbc80411c74ab74ffd055885322c325aa3d1c404b29c3fd70a737e86083eecae58ef394db1cb56bc122d06cff63742aa89a8e868730c64
988024fe37 test: add check_node_connections in util (brunoerg)
Pull request description:
This function (`check_node_connections`) exists in `feature_anchors.py` and `p2p_add_connections.py` and does the same thing in both, this PR moves this function to util to avoid repetition and facilitate maintenance.
ACKs for top commit:
laanwj:
Code review ACK 988024fe37
Tree-SHA512: bf86c5659933539c72cb91ad587552b45c918be74d36fb429e78f3b954f01ed0855a85dd49aea35b432fbd18227c05eb3fec8b99c139c3509c39b19bccf6b7fd
The previous diff touched most files in ./test/, so bump the headers to
avoid having to touch them again for a bump later.
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./test/
-END VERIFY SCRIPT-
facc352648 test: Implicitly sync after generate*, unless opted out (MarcoFalke)
Pull request description:
The most frequent failure in functional tests are intermittent races. Fixing such bugs is cumbersome because it involves:
* Noticing the failure
* Fetching and reading the log to determine the test case that failed
* Adding a `self.sync_all()` where it was forgotten
* Spamming out a pr and waiting for review, which is already sparse
Also, writing a linter to catch those is not possible, nor is review effective in finding these bugs prior to merge.
Fix all future intermittent races caused by a missing sync_block call by calling `sync_all` implicitly after each `generate*`, unless opted out. This ensures that the code is race-free (with regards to blocks) when the tests pass once, instead of our current approach where the code can never be guaranteed to be race-free.
There are some scripted-diff cleanups (see https://github.com/bitcoin/bitcoin/pull/22567), but they will be submitted in a follow-up to reduce the conflicts in this pull.
ACKs for top commit:
lsilva01:
tACK facc352 on Ubuntu 20.04
brunoerg:
tACK facc352648 on MacOS 11.6
Tree-SHA512: 046a40a066b4a3bd28a3077bd654fa8887442dd1f0ec6fd11671865809ef02376f126eb667a1320ebd67b6e372c78c00dbf8bd25d86ed86f1d9a25363103ed97
Because of floating point precision issues, not all of the rounding done
is always correct. To fix this, the fee calculation for
assert_fee_amount is changed to better reflect how CFeeRate::GetFee does
it.
First the feerate is converted to an int representing sat/kvb. Then this
is multiplied by the transaction size, divivided by 1000, and rounded up
to the nearest sat. The result is then converted back to BTC (divided by
1e8) and then rounded down to the nearest sat to avoid precision errors.
b658d7d5c5 test: update assert_fee_amount() in test_framework/util.py (Jon Atack)
Pull request description:
Follow-up to 42e1b5d979 (#12486).
- update call to `round()` with our utility function `satoshi_round()` to avoid intermittent test failures
- rename `fee_per_kB` to `feerate_BTC_kvB` for precision
- store division result in `feerate_BTC_vB`
Possibly resolves#19418.
ACKs for top commit:
meshcollider:
utACK b658d7d5c5
Tree-SHA512: f124ded98c913f98782dc047a85a05d3fdf5f0585041fa81129be562138f6261ec1bd9ee2af89729028277e75b591b0a7ad50244016c2b2fa935c6e400523183
- update call to round() with satoshi_round() to avoid intermittent test failures
- rename fee_per_kB to feerate_BTC_kvB for precision
- store division result in feerate_BTC_vB
This avoids having to remember to set it whenever mocktime is used with
peer connections. Also, it might help avoiding disconnects when
attaching a debugger to a running test.
8ca51af1ec test: Disable automatic connections by default (Martin Zumsande)
Pull request description:
A node normally doesn't make automatic connections to peers in the functional tests because neither DNS seeds nor hardcoded peers are available on regtest. However, when random entries are inserted into addrman as part of a functional test (e.g. while testing addr relay), `ThreadOpenConnections` will periodically try to connect to them, resulting in log entries such as:
`[opencon] [net.cpp:400] [ConnectNode] trying connection 18.166.1.1:8333 lastseen=0.0hrs`
I don't think it's desirable that functional tests try to connect to random computers on the internet, aside from the possibility that at some point in time someone out there might actually answer in a way to ruin a test.
This PR fixes this problem by disabling `ThreadOpenConnections` by adding `-connect=0` to the default args, and adding exceptions only when needed for the test to pass.
ACKs for top commit:
tryphe:
Concept ACK, light code review ACK 8ca51af1ec
Tree-SHA512: bcfb2de610e6c35a97a2bd7ad6267e968b1ac7529638d99276993cd5bc93ce9919d54e22d6dc84e1b02ecd626ab6554e201693552ea065c29794eece38c43f7d
fbeb8c43bc test: add type annotations to util.get_rpc_proxy (fanquake)
Pull request description:
Split out from #22092 while we address the functional test failure.
ACKs for top commit:
instagibbs:
ACK fbeb8c43bc
Tree-SHA512: 031ef8703202ae5271787719fc3fea8693574b2eb937ccf852875de95798d7fa3c39a8db7c91993d0c946b45d9b4d6de570bd1102e0344348784723bd84803a8
This prevents the node from trying to connect to random IPs on the internet
while running the functional tests. Exceptions are added when required for
the test to pass.
Since we want tests to run quickly, and since tests do a lot more db
operations than expected we expect to see in actual usage, we disable
sqlite's syncing behavior to make db operations run much faster. This
syncing behavior is necessary for normal operation as it helps guarantee
that data won't become lost or corrupted, but in tests, we don't care
about that.
1) add a new sane "address" field (for outputs that have an
identifiable address, which doesn't include bare multisig)
2) with -deprecatedrpc: leave "reqSigs" and "addresses" intact
(with all weird/wrong behavior they have now)
3) without -deprecatedrpc: drop "reqSigs" and "addresses" entirely,
always.
ff44cae279 test: Change feature_config_args.py not to rely on strange regtest=0 behavior (Russell Yanofsky)
Pull request description:
Update test to simply generate a normal mainnet configuration file instead of using a crazy setup where a regtest=1 config file using an includeconf in the [regtest] section includes another config file that specifies regtest=0, retroactively switching the network to mainnet.
This setup was fragile and only worked because the triggered InitError happened early enough that none of the ignored [regtest] options mattered (only affecting log output).
This change was originally made as part of #17493
Top commit has no ACKs.
Tree-SHA512: 3f77305454f04438493dfc2abd78a00434b30869454d1c3f54587b9c1f63239c49c90fb3b4d3a777ad130f2184e0f2dac87cee4cd23c50f1b3496a375943da01
5f9c0b6360 wallet: Remove -upgradewallet from dummywallet (MarcoFalke)
a314271f08 test: Remove unused wallet.dat (MarcoFalke)
bf7635963c tests: Test specific upgradewallet scenarios and that upgrades work (Andrew Chow)
4b418a9dec test: Add test_framework/bdb.py module for inspecting bdb files (Andrew Chow)
092fc43485 tests: Add a sha256sum_file function to util (Andrew Chow)
0bd995aa19 wallet: upgrade the CHDChain version number when upgrading to split hd (Andrew Chow)
8e32e1c41c wallet: remove nWalletMaxVersion (Andrew Chow)
bd7398cc62 wallet: have ScriptPubKeyMan::Upgrade check against the new version (Andrew Chow)
5f720544f3 wallet: Add GetClosestWalletFeature function (Andrew Chow)
842ae3842d wallet: Add utility method for CanSupportFeature (Andrew Chow)
Pull request description:
This PR cleans up the wallet upgrade mechanism a bit, fixes some probably bugs, and adds more test cases.
The `nWalletMaxVersion` member variable has been removed as it made `CanSupportFeature` unintuitive and was causing a couple of bugs. The reason this was introduced originally was to allow a wallet upgrade to only occur when the new feature is first used. While this makes sense for the old `-upgradewallet` option, for an RPC, this does not quite make sense. It's more intuitive for an upgrade to occur if possible if the `upgradewallet` RPC is used as that's an explicit request to upgrade a particular wallet to a newer version. `nWalletMaxVersion` was only relevant for upgrades to `FEATURE_WALLETCRYPT` and `FEATURE_COMPRPUBKEY` both of which are incredibly old features. So for such wallets, the behavior of `upgradewallet` will be that the feature is enabled immediately without the wallet needing to be encrypted at that time (note that `FEATURE_WALLETCRYPT` indicates support for encryption, not that the wallet is encrypted) or for a new key to be generated.
`CanSupportFeature` would previously indicate whether we could upgrade to `nWalletMaxVersion` not just whether the current wallet version supported a feature. While this property was being used to determine whether we should upgrade to HD and HD chain split, it was also causing a few bugs. Determining whether we should upgrade to HD or HD chain split is resolved by passing into `ScriptPubKeyMan::Upgrade` the version we are upgrading to and checking against that. By removing `nWalletMaxVersion` we also fix a bug where you could upgrade to HD chain split without the pre-split keypool.
`nWalletMaxVersion` was also the version that was being reported by `getwalletinfo` which meant that the version reported was not always consistent across restarts as it depended on whether `upgradewallet` was used. Additionally to make the wallet versions consistent with actually supported versions, instead of just setting the wallet version to whatever is given to `upgradewallet`, we normalize the version number to the closest supported version number. For example, if given 150000, we would store and report 139900.
Another bug where CHDChain was not being upgraded to the version supporting HD chain split is also fixed by this PR.
Lastly several more tests have been added. Some refactoring to the test was made to make these tests easier. These tests check specific upgrading scenarios, such as from non-HD (version 60000) to HD to pre-split keypool. Although not specifically related to `upgradewallet`, `UpgradeKeyMetadata` is now being tested too.
Part of the new tests is checking that the wallet files are identical before and after failed upgrades. To facilitate this, a utility function `sha256sum_file` has been added. Another part of the tests is to examine the wallet file itself to ensure that the records in the wallet.dat file have been correctly modified. So a new `bdb.py` module has been added to deserialize the BDB db of the wallet.dat file. This format isn't explicitly documented anywhere, but the code and comments in BDB's source code in file `dbinc/db_page.h` describe it. This module just dumps all of the fields into a dict.
ACKs for top commit:
MarcoFalke:
approach ACK 5f9c0b6360
laanwj:
Code review ACK 5f9c0b6360
jonatack:
ACK 5f9c0b6360, approach seems fine, code review, only skimmed the test changes but they look well done, rebased on current master, debug built and verified the `wallet_upgradewallet.py` test runs green both before and after running `test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2`
Tree-SHA512: 7c4ebf420850d596a586cb6dd7f2ef39c6477847d12d105fcd362abb07f2a8aa4f7afc5bfd36cbc8b8c72fcdd1de8d2d3f16ad8e8ba736b6f4f31f133fe5feba
