This uses a regtest-only RPC which checks that the chain is literally
"regtest". Since regtest is disabled in Elements (we use elementsregtest)
I weakened the check for this to just check that the chain name has
"regtest" somewhere in it. Hopefully this isn't too magical.
Everyone is _not_ connected in this test, so on slow machines
where this check triggers (e.g. the CI boxes) the test incorrectly
fails. This was also a source of (very infrequent) spurious failures
during the rebase.
Several conflicts in the C++ code related to the new `flags` parameter
to `CheckSignature` and the corresponding function being renamed upstream
to `CheckSignatureECDSA`.
Several conflicts in the test harness as Steven sorta pulled the new
upstream ECKey module into the Python code, and the actual upstream
code was slightly different. Also needed to update the feature_taproot
code to always use the non-RANGEPROOF sighash since dynafed is not
enabled in the Taproot test.
Also had to pull the `set_wif` method out of `ECKey` and inline it because
otherwise it triggers a "circular inclusion" error between script.py (which
would pull in `base58_to_bytes` from address.py) and address.py (which now
pulls in some taproot EC related stuff from script.py).
Noticed that #960 does not test the "sighash rangeproof flag set but no
witnesses" case.
Somehow the CECKey impl is really broken if you want to set
a raw private key. Core master already has a new impl so I just
took that one and replaces the usages. This will probably be equivalent
with what would be rebased in at a later stage.
b4dd2ef800 [test] Test the add_outbound_p2p_connection functionality (Amiti Uttarwar)
602e69e427 [test] P2PBlocksOnly - Test block-relay-only connections. (Amiti Uttarwar)
8bb6beacb1 [test/refactor] P2PBlocksOnly - Extract transaction violation test into helper. (Amiti Uttarwar)
99791e7560 [test/refactor] P2PBlocksOnly - simplify transaction creation using blocktool helper. (Amiti Uttarwar)
3997ab9154 [test] Add test framework support to create outbound connections. (Amiti Uttarwar)
5bc04e8837 [rpc/net] Introduce addconnection to test outbounds & blockrelay (Amiti Uttarwar)
Pull request description:
The existing functional test framework uses the `addnode` RPC to spin up manual connections between bitcoind nodes. This limits our ability to add integration tests for our networking code, which often executes different code paths for different connection types.
**This PR enables creating `outbound` & `block-relay-only` P2P connections in the functional tests.** This allows us to increase our p2p test coverage, since we can now verify expectations around these connection types.
This builds out the [prototype](https://github.com/bitcoin/bitcoin/issues/14210#issuecomment-527421978) proposed by ajtowns in #14210. 🙌🏽
An overview of this branch:
- introduces a new test-only RPC function `addconnection` which initiates opening an `outbound` or `block-relay-only` connection. (conceptually similar to `addnode` but for different connection types & restricted to regtest)
- adds `test_framework` support so a mininode can open an `outbound`/`block-relay-only` connection to a `P2PInterface`/`P2PConnection`.
- updates `p2p_blocksonly` tests to create a `block-relay-only` connection & verify expectations around transaction relay.
- introduces `p2p_add_connections` test that checks the behaviors of the newly introduced `add_outbound_p2p_connection` test framework function.
With these changes, there are many more behaviors that we can add integration tests for. The blocksonly updates is just one example.
Huge props to ajtowns for conceiving the approach & providing me feedback as I've built out this branch. Also thank you to jnewbery for lots of thoughtful input along the way.
ACKs for top commit:
troygiorshev:
reACK b4dd2ef800
jnewbery:
utACK b4dd2ef800
MarcoFalke:
Approach ACK b4dd2ef800🍢
Tree-SHA512: d1cba768c19c9c80e6a38b1c340cc86a90701b14772c4a0791c458f9097f6a4574b4a4acc7d13d6790c7b1f1f197e2c3d87996270f177402145f084ef8519a6b
faabc26a61 test: Replace getmempoolentry with testmempoolaccept in MiniWallet (MarcoFalke)
Pull request description:
This is a refactor to not use the return value of `sendrawtransaction` and `getmempoolentry` with the goal that submitting the tx to the mempool will become optional.
ACKs for top commit:
mjdietzx:
ACK faabc26a61
Tree-SHA512: 4260a59e65fed1c807530dad23f1996ba6e881bcce91995f5b498a0be6001f67b3e0251507c8801750fe105410147c68bb2f393ebe678c6a3a4d045e5d72fc19
a191e23b8e doc: Add release notes (Hennadii Stepanov)
ae749d12dd doc: Add libnatpmp stuff (Hennadii Stepanov)
e28f9be87a ci: Add libnatpmp-dev package to some builds (Hennadii Stepanov)
5a0185b6c9 gui: Add NAT-PMP network option (Hennadii Stepanov)
a39f7336a3 net: Add -natpmp command line option (Hennadii Stepanov)
28acffd9d5 net: Add NAT-PMP to port mapping loop (Hennadii Stepanov)
a8d9f275d0 net: Add libnatpmp support (Hennadii Stepanov)
58e8364dcd gui: Apply port mapping changes on dialog exit (Hennadii Stepanov)
cf151cc68c scripted-diff: Rename UPnP stuff (Hennadii Stepanov)
4e91b1e24d net: Add flags for port mapping protocols (Hennadii Stepanov)
8b50d1b5bb net: Keep trying to use UPnP when -upnp=1 (Hennadii Stepanov)
28e2961fd6 refactor: Replace magic number with named constant (Hennadii Stepanov)
02ccf69dd6 refactor: Move port mapping code to its own module (Hennadii Stepanov)
Pull request description:
Close#11902
This PR is an alternative to:
- #12288
- #15717
To compile with NAT-PMP support on Ubuntu [`libnatpmp-dev`](https://packages.ubuntu.com/source/bionic/libnatpmp) should be available.
Log excerpt:
```
2020-02-05T20:12:28Z [mapport] NAT-PMP: public address = 95.164.65.194
2020-02-05T20:12:28Z [mapport] AddLocal(95.164.65.194:18333,3)
2020-02-05T20:12:28Z [mapport] NAT-PMP: port mapping successful.
```
See: [`libnatpmp`](https://miniupnp.tuxfamily.org/libnatpmp.html)
---
Some follow-ups are out of this PR's scope:
- mention NAT-PMP library in the version message
- ~integrate NAT-PMP into the GUI~ (already [added](https://github.com/bitcoin/bitcoin/pull/18077#issuecomment-589405068))
ACKs for top commit:
laanwj:
Tested and code review ACK a191e23b8e
Tree-SHA512: 10e19267c21bf30f20ff1abfc882d526049f0e790b95e12f109dc2bed7c0aef45de03eaf967f4e667e7509be04f1873a5c508087393d947205f3aab2ad6d7cf1
In the interest of increasing our P2P test coverage, add support to create
full-relay or block-relay-only connections. To support this, a P2P connection
spins up a listening thread & uses a callback to trigger the node initiating
the connection.
Co-authored-by: Anthony Towns <aj@erisian.com.au>
9815332d51 test: Change MuHash Python implementation to match cpp version again (Fabian Jahr)
01297fb3ca fuzz: Add MuHash consistency fuzz test (Fabian Jahr)
b111410914 test: Add MuHash3072 fuzz test (Fabian Jahr)
c122527385 bench: Add Muhash benchmarks (Fabian Jahr)
7b1242229d test: Add MuHash3072 unit tests (Fabian Jahr)
adc708c98d crypto: Add MuHash3072 implementation (Fabian Jahr)
0b4d290bf5 crypto: Add Num3072 implementation (Fabian Jahr)
589f958662 build: Check for 128 bit integer support (Fabian Jahr)
Pull request description:
This is the first split of #18000 which implements the Muhash algorithm and uses it to calculate the UTXO set hash in `gettxoutsetinfo`.
ACKs for top commit:
laanwj:
Code review ACK 9815332d51
Tree-SHA512: 4bc090738f0e3d80b74bdd8122e24a8ce80121120fd37c7e4335a73e7ba4fcd7643f2a2d559e2eebf54b8e3a3bd5f12cfb27ba61ded135fda210a07a233eae45
40fdb2a212 test: Fix Comment Typo in BitcoinTestFramework (Joel Klabo)
Pull request description:
Missing "override" in comment describing use of set_test_params
ACKs for top commit:
michaelfolkson:
ACK 40fdb2a212
Tree-SHA512: bf893a0d5f8dc86a3ec2eaf48cd7c0f0f832f3b3d254b3d99953336db7e294571b1d2c8686030bf8a27cbe67b1a85a54e53ebefb2e57d6d8d6ac864a15dce4e7
812baaa1f8 Switch to BIP341's suggested scheme for outputs without script (Pieter Wuille)
Pull request description:
BIP341 suggests using Hash<sub>TapTweak</sub>(pubkey) to derive the tweak in case of key-only outputs. The functional test framework currently uses Hash<sub>TapTweak</sub>(pubkey || 0x00...00) instead. Change this.
There is no technical reason to prefer one over the other, but in case someone looks at it for inspiration, it's better to be consistent with the BIP.
ACKs for top commit:
laanwj:
ACK 812baaa1f8
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/20189/commits/812baaa1f804a8c848284759c75e5f9d19e942f8
Tree-SHA512: 02576c38776ec786255f49d7edecdb1ed8a9dcf0f547d58c23099588b4c3296edf279b103a6eb80e0f07d3c5ee9743f67d152f5244fd63adc6613b004f6969ed
bc4a230087 Remove redundant p2p lock tacking for tx download functional tests (Antoine Riard)
d3b5eac9a9 Add mutation for functional test test_preferred_inv (Antoine Riard)
06efb3163c Add functional test test_txid_inv_delay (Antoine Riard)
a07910abcd test: Makes wtxidrelay support a generic P2PInterface option (Antoine Riard)
Pull request description:
This is a simple functional test to increase coverage of #19988, checking that txid announcements from txid-relay peers are delayed by TXID_RELAY_DELAY, assuming we have at least another wtxid-relay peer.
You can verify new test with the following diff :
```
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index f14db379f..2a2805df5 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -773,7 +773,7 @@ void PeerManager::AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, std
auto delay = std::chrono::microseconds{0};
const bool preferred = state->fPreferredDownload;
if (!preferred) delay += NONPREF_PEER_TX_DELAY;
- if (!gtxid.IsWtxid() && g_wtxid_relay_peers > 0) delay += TXID_RELAY_DELAY;
+ //if (!gtxid.IsWtxid() && g_wtxid_relay_peers > 0) delay += TXID_RELAY_DELAY;
const bool overloaded = !node.HasPermission(PF_RELAY) &&
m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT;
if (overloaded) delay += OVERLOADED_PEER_TX_DELAY;
```
ACKs for top commit:
laanwj:
ACK bc4a230087
Tree-SHA512: 150e806bc5289feda94738756ab375c7fdd23c80c12bd417d3112043e26a91a717dc325a01079ebd02a88b90975ead5bd397ec86eb745c7870ebec379a8aa711
fa40168ab3 Remove unused bits from service flags enum (MarcoFalke)
Pull request description:
Remove service bits that haven't been observed on the active network for years and won't ever be observed on the network with this meaning. Keeping this dead assignment in our source code forever doesn't add any value.
I somehow forgot to do this in commit fa0d0ff6e1.
ACKs for top commit:
laanwj:
Code review ACK fa40168ab3
practicalswift:
cr ACK fa40168ab3
fanquake:
ACK fa40168ab3
Tree-SHA512: 376e5ac05940493cf2209fea60515c843e978c4b476f2524f6bf7a37a646d237c3ddcf6c0fa23641f9ba550f625609703d9b51b4be631a7f2a90e1092b557232
This one was pretty brutal, but ultimately nothing too surprising.
Just had to keep track of which wallets owned which blinding keys,
and in wallet_importprunedfunds.py I had to manually create a
Liquid address.
