aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2022-05-30Swap out hashlib.ripemd160 for own implementationPieter Wuille
Github-Pull: 23716 Rebased-From: 5b559dc7ecf37ab1604b75ec8ffe8436377a5fb1
2022-05-30Add pure Python RIPEMD-160Pieter Wuille
Github-Pull: 23716 Rebased-From: ad3e9e1f214d739e098c6ebbd300da5df1026a44
2022-03-05fs: Make compatible with boost 1.78Andrew Chow
Github-Pull: #24104 Rebased-From: dc5d6b0d4793ca978f71f69ef7d6b818794676c2
2022-02-15test: Call ceildiv helper with integerMartin Zumsande
It returns an incorrect result when called with a Decimal, for which the "//" operator works differently. Also drop unnecessary call to satoshi_round. Github-Pull: #24239 Rebased-From: d1fab9d5d27a2db2546db0f610e0f6929ec4864e
2022-02-15tests: Calculate fees more similarly to CFeeRate::GetFeeAndrew Chow
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. Github-Pull: #22949 Rebased-From: 80dc829be7f8c3914074b85bb4c125baba18cb2c
2022-02-15tests: Test for assertion when feerate is rounded downAndrew Chow
When calculating a txs absolute fee, if the fee is rounded down to the nearest satoshi, it is possible for the coin selection algorithms to undercalculate the fee needed. This can lead to an assertion error in some situations. One such scenario is added to rpc_fundrawtransaction.py. Github-Pull: #22949 Rebased-From: ce2cc44afd51f3df4ee7f14ea05b8da229183923
2022-02-15fees: Always round up fee calculated from a feerateAndrew Chow
When calculating the fee for a given tx size from a fee rate, we should always round up to the next satoshi. Otherwise, if we round down (via truncation), the calculated fee may result in a fee with a feerate slightly less than targeted. This is particularly important for coin selection as a slightly lower feerate than expected can result in a variety of issues. Github-Pull: #22949 Rebased-From: 0fbaef9676a1dcb84bcf95afd8d994831ab327b6
2022-02-15Fix (inverse) meaning of -persistmempoolMarcoFalke
Github-Pull: #23061 Rebased-From: faff17bbde6dcb1482a6210bc48b3192603a446f
2021-08-20test: fix bug in 22686S3RK
Github-Pull: bitcoin/bitcoin#22742 Rebased-From: 8dcbbbea6486e9ab7d5e7397b82585141f9910bf
2021-08-20test: Test for ApproximateBestSubset edge case with too little feesAndrew Chow
ApproximateBestSubset had an edge case (due to not using GetSelectionAmount) where it was possible for it to return success but fail to select enough to cover transaction fees. A test is added that could trigger this failure prior to the fix being implemented. Github-Pull: bitcoin/bitcoin#22686 Rebased-From: 92885c4f69a5e6fc4989677d6e5be8a666fbff0d
2021-07-20Merge bitcoin/bitcoin#22492: wallet: Reorder locks in dumpwallet to avoid ↵MarcoFalke
lock order assertion 9b85a5e2f7e003ca8621feaac9bdd304d19081b4 tests: Test for dumpwallet lock order issue (Andrew Chow) 25d99e6511d8c43b2025a89bcd8295de755346a7 Reorder dumpwallet so that cs_main functions go first (Andrew Chow) Pull request description: When a wallet is loaded which has an unconfirmed transaction in the mempool, it will end up establishing the lock order of cs_wallet -> cs_main -> cs_KeyStore. If `dumpwallet` is used on this wallet, then a lock order of cs_wallet -> cs_KeyStore -> cs_main will be used, which causes a lock order assertion. This PR fixes this by reordering `dumpwallet` and `GetKeyBirthTimes` (only used by `dumpwallet`). Specifically, in both functions, the function calls which lock cs_main are done prior to locking cs_KeyStore. This avoids the lock order issue. Additionally, I have added a test case to `wallet_dump.py`. Of course testing this requires `--enable-debug`. Fixes #22489 ACKs for top commit: MarcoFalke: review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4 🎰 ryanofsky: Code review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4. Nice to reduce lock scope, and good test! prayank23: tACK https://github.com/bitcoin/bitcoin/pull/22492/commits/9b85a5e2f7e003ca8621feaac9bdd304d19081b4 lsilva01: Tested ACK https://github.com/bitcoin/bitcoin/pull/22492/commits/9b85a5e2f7e003ca8621feaac9bdd304d19081b4 under the same conditions reported in issue #22489 and the `dumpwallet` command completed successfully. Tree-SHA512: d370a8f415ad64ee6a538ff419155837bcdbb167e3831b06572562289239028c6b46d80b23d227286afe875d9351f3377574ed831549ea426fb926af0e19c755
2021-07-20Merge bitcoin/bitcoin#22261: [p2p/mempool] Two small fixes to node broadcast ↵fanquake
logic 5a77abd4e657458852875a07692898982f4b1db5 [style] Clean up BroadcastTransaction() (John Newbery) 7282d4c0363ab5152baa34af626cb49afbfddc32 [test] Allow rebroadcast for same-txid-different-wtxid transactions (glozow) cd48372b67d961fe661990a2c6d3cc3d91478924 [mempool] Allow rebroadcast for same-txid-different-wtxid transactions (John Newbery) 847b6ed48d7bacec9024618922e9b339d2d97676 [test] Test transactions are not re-added to unbroadcast set (Duncan Dean) 2837a9f1eaa2c6bf402d1d9891d9aa84c4a56033 [mempool] Only add a transaction to the unbroadcast set when it's added to the mempool (John Newbery) Pull request description: 1. Only add a transaction to the unbroadcast set when it's added to the mempool Currently, if BroadcastTransaction() is called to rebroadcast a transaction (e.g. by ResendWalletTransactions()), then we add the transaction to the unbroadcast set. That transaction has already been broadcast in the past, so peers are unlikely to request it again, meaning RemoveUnbroadcastTx() won't be called and it won't be removed from m_unbroadcast_txids. Net processing will therefore continue to attempt rebroadcast for the transaction every 10-15 minutes. This will most likely continue until the node connects to a new peer which hasn't yet seen the transaction (or perhaps indefinitely). Fix by only adding the transaction to the broadcast set when it's added to the mempool. 2. Allow rebroadcast for same-txid-different-wtxid transactions There is some slightly unexpected behaviour when: - there is already transaction in the mempool (the "mempool tx") - BroadcastTransaction() is called for a transaction with the same txid as the mempool transaction but a different witness (the "new tx") Prior to this commit, if BroadcastTransaction() is called with relay=true, then it'll call RelayTransaction() using the txid/wtxid of the new tx, not the txid/wtxid of the mempool tx. For wtxid relay peers, in SendMessages(), the wtxid of the new tx will be taken from setInventoryTxToSend, but will then be filtered out from the vector of wtxids to announce, since m_mempool.info() won't find the transaction (the mempool contains the mempool tx, which has a different wtxid from the new tx). Fix this by calling RelayTransaction() with the wtxid of the mempool transaction in this case. The third commit is a comment/whitespace only change to tidy up the BroadcastTransaction() function. ACKs for top commit: duncandean: reACK 5a77abd naumenkogs: ACK 5a77abd4e657458852875a07692898982f4b1db5 theStack: re-ACK 5a77abd4e657458852875a07692898982f4b1db5 lsilva01: re-ACK https://github.com/bitcoin/bitcoin/pull/22261/commits/5a77abd4e657458852875a07692898982f4b1db5 Tree-SHA512: d1a46d32a9f975220e5b432ff6633fac9be01ea41925b4958395b8d641680500dc44476b12d18852e5b674d2d87e4d0160b4483e45d3d149176bdff9f4dc8516
2021-07-20Merge bitcoin/bitcoin#22096: p2p: AddrFetch - don't disconnect on ↵fanquake
self-announcements 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 test: Add functional test for AddrFetch connections (Martin Zumsande) c34ad3309f93979b274a37de013502b05d25fad8 net, rpc: Enable AddrFetch connections for functional testing (Martin Zumsande) 533500d9072b7d5a36a6491784bdeb9247e91fb0 p2p: Add timeout for AddrFetch peers (Martin Zumsande) b6c5d1e450dde6a54bd785504c923adfb45c7060 p2p: AddrFetch - don't disconnect on self-announcements (Martin Zumsande) Pull request description: AddrFetch connections (old name: oneshots) are intended to be short-lived connections on which we ask a peer for addresses via `getaddr` and disconnect after receiving them. This is done by disconnecting after receiving the first `addr`. However, it is no longer working as intended, because nowadays, the first `addr` a typical bitcoin core node sends is its self-announcement. So we'll disconnect before the peer gets a chance to answer our `getaddr`. I checked that this affects both `-seednode` peers specified manually, and DNS seeds when AddrFetch is used as a fallback if DNS doesn't work for us. The current behavior of getting peers via AddrFetch when starting with an empty addrman would be to connect to the peer, receive its self-announcement and add it to addrman, disconnect, reconnect to the same peer again as a full outbound (no other addresses in addrman) and then receive more `addr`. This is silly and not in line with AddrFetch peer being intended to be short-lived peers.  Fix this by only disconnecting after receiving an `addr` message of size > 1. [Edit] As per review discussion, this PR now also adds a timeout after which we disconnect if we haven't received any suitable `addr`, and a functional test. ACKs for top commit: amitiuttarwar: reACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 naumenkogs: ACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 jnewbery: ACK 5730a43703 Tree-SHA512: 8a81234f37e827705138eb254223f7f3b3bf44a06cb02126fc7990b0d231b9bd8f07d38d185cc30d55bf35548a6fdc286b69602498d875b937e7c58332158bf9
2021-07-20Merge bitcoin/bitcoin#21711: guix: Add full installation and usage documentationfanquake
fac4814106c796b8786dd90053513cc35142dfe5 doc/release-process: Add torrent creation details (Carl Dong) 5d24cc3d82dad6812f8370c3ccc7c2b5a6c12c11 guix/INSTALL: Guix installs init scripts in libdir (Carl Dong) 5da2ee49d5b44de803b671aedbdd14e5c1d71ea9 guix/INSTALL: Add coreutils/inotify-dir-recreate troubleshooting (Carl Dong) 318c60700b7bbb7ec09a29bf037e7c2787646be6 guix: Adapt release-process.md to new Guix process (Carl Dong) fcab35b2292f9221eaba521740e8b3b2511a8b78 guix-attest: Produce and sign normalized documents (Carl Dong) c2541fd0ca99481a5a792a8f2772925d64fb0491 guix: Overhaul README (Carl Dong) 46ce6ce3782dfbd8f9d26dc2ba0f284755e75f2d tree-wide: Rename gitian-keys to builder-keys (Carl Dong) fc4f8449f34e32b0b9ac9d218d6c3264b02467ba guix: Update various check_tools lists (Carl Dong) 263220a85c1df218431fafbda07c8b23ccc4ce4d guix: Check for a sane services database (Carl Dong) Pull request description: Based on: #21462 Keeping the README in one file so that it's easy to search through. Will add more jumping links later so navigation is easier. Current TODOs: - [x] Shell installer option: prompt user to re-login for `/etc/profile.d` entry to be picked up - [x] Binary tarball option: prompt user to create `/etc/profile.d` entry and re-login - [x] Fanquake docker option: complete section - [x] Arch Linux AUR option: prompt to start `guix-daemon-latest` unit after finishing "optional setup" section - [x] Building from source option: Insert dependency tree diagram that I made - [x] Building from source option: redo sectioning, kind of a mess right now - [x] Optional setup: make clear which parts are only needed if building from source - [x] Workaround 1 for GnuTLS: perhaps mention how to remove Guix build farm's key - [x] Overall (after everything): Make the links work. Note to self: wherever possible, tell user how to check that something is true rather than branching by installation option. ACKs for top commit: fanquake: ACK fac4814106c796b8786dd90053513cc35142dfe5 - going to go ahead and merge this now. It's a lot of documentation, and could probably be nit-picked / improved further, however, that can continue over the next few weeks. I'm sure more (backportable) improvements / clarifications will be made while we progress through RCs towards a new release. Tree-SHA512: dc46c0ecdfc67c7c7743ca26e4a603eb3f54adbf81be2f4c1f4c20577ebb84b5250b9c9ec89c0e9860337ab1c7cff94d7963c603287267deecfe1cd987fa070a
2021-07-19tests: Test for dumpwallet lock order issueAndrew Chow
Adds a test for the condition which can trigger a lock order assertion. Specifically, there must be an unconfirmed transaction in the mempool which belongs to the wallet being loaded. This will establish the order of cs_wallet -> cs_main -> cs_KeyStore. Then dumpwallet is called on that wallet. Previously, this would have used a lock order of cs_wallet -> cs_KeyStore -> cs_main, but this should be fixed now. The test ensures that.
2021-07-19Merge bitcoin/bitcoin#22387: Rate limit the processing of rumoured addressesW. J. van der Laan
a4bcd687c934d47aa3922334e97e579caf5f8124 Improve tests using statistics (John Newbery) f424d601e1b6870e20bc60f5ccba36d2e210377b Add logging and addr rate limiting statistics (Pieter Wuille) b4ece8a1cda69cc268d39d21bba59c51fa2fb9ed Functional tests for addr rate limiting (Pieter Wuille) 5648138f5949013331c017c740646e2f4013bc24 Randomize the order of addr processing (Pieter Wuille) 0d64b8f709b4655d8702f810d4876cd8d96ded82 Rate limit the processing of incoming addr messages (Pieter Wuille) Pull request description: The rate at which IP addresses are rumoured (through ADDR and ADDRV2 messages) on the network seems to vary from 0 for some non-participating nodes, to 0.005-0.025 addr/s for recent Bitcoin Core nodes. However, the current codebase will happily accept and process an effectively unbounded rate from attackers. There are measures to limit the influence attackers can have on the addrman database (bucket restrictions based on source IPs), but still - there is no need to permit them to feed us addresses at a rate that's orders of magnitude larger than what is common on the network today, especially as it will cause us to spam our peers too. This PR implements a [token bucket](https://en.wikipedia.org/wiki/Token_bucket) based rate limiter, allowing an average of 0.1 addr/s per connection, with bursts up to 1000 addresses at once. Whitelisted peers as well as responses to GETADDR requests are exempt from the limit. New connections start with 1 token, so as to not interfere with the common practice of peers' self-announcement. ACKs for top commit: laanwj: ACK a4bcd687c934d47aa3922334e97e579caf5f8124 vasild: ACK a4bcd687c934d47aa3922334e97e579caf5f8124 jnewbery: ACK a4bcd687c934d47aa3922334e97e579caf5f8124 jonatack: ACK a4bcd687c934d47aa3922334e97e579caf5f8124 Tree-SHA512: b757de76ad78a53035b622944c4213b29b3b55d3d98bf23585afa84bfba10808299d858649f92269a16abfa75eb4366ea047eae3216f7e2f6d3c455782a16bea
2021-07-16Test that descriptor wallet upgrade does nothingAndrew Chow
2021-07-15Improve tests using statisticsJohn Newbery
2021-07-15Functional tests for addr rate limitingPieter Wuille
2021-07-15Randomize the order of addr processingPieter Wuille
2021-07-15Rate limit the processing of incoming addr messagesPieter Wuille
While limitations on the influence of attackers on addrman already exist (affected buckets are restricted to a subset based on incoming IP / network group), there is no reason to permit them to let them feed us addresses at more than a multiple of the normal network rate. This commit introduces a "token bucket" rate limiter for the processing of addresses in incoming ADDR and ADDRV2 messages. Every connection gets an associated token bucket. Processing an address in an ADDR or ADDRV2 message from non-whitelisted peers consumes a token from the bucket. If the bucket is empty, the address is ignored (it is not forwarded or processed). The token counter increases at a rate of 0.1 tokens per second, and will accrue up to a maximum of 1000 tokens (the maximum we accept in a single ADDR or ADDRV2). When a GETADDR is sent to a peer, it immediately gets 1000 additional tokens, as we actively desire many addresses from such peers (this may temporarily cause the token count to exceed 1000). The rate limit of 0.1 addr/s was chosen based on observation of honest nodes on the network. Activity in general from most nodes is either 0, or up to a maximum around 0.025 addr/s for recent Bitcoin Core nodes. A few (self-identified, through subver) crawler nodes occasionally exceed 0.1 addr/s.
2021-07-15Merge bitcoin/bitcoin#22211: net: relay I2P addresses even if not reachable ↵W. J. van der Laan
(by us) 7593b06bd1262f438bf34769ecc00e9c22328e56 test: ensure I2P addresses are relayed (Vasil Dimov) e7468139a1dddd4946bc70697ec38816b3fa8f9b test: make CAddress in functional tests comparable (Vasil Dimov) 33e211d2a442e4555ff3401f92af4ee2f7552568 test: implement ser/unser of I2P addresses in functional tests (Vasil Dimov) 86742811ce3662789ac85334008090a3b54babe3 test: use NODE_* constants instead of magic numbers (Vasil Dimov) ba45f0270815d54ae3290efc16324c2ff1984565 net: relay I2P addresses even if not reachable (by us) (Vasil Dimov) Pull request description: Nodes that can reach the I2P network (have set `-i2psam=`) will relay I2P addresses even without this patch. However, nodes that can't reach the I2P network will not. This was done as a precaution in https://github.com/bitcoin/bitcoin/pull/20119 before anybody could connect to I2P because then, for sure, it would have been useless. Now, however, we have I2P support and a bunch of I2P nodes, so get all nodes on the network to relay I2P addresses to help with propagation, similarly to what we do with Tor addresses. ACKs for top commit: jonatack: ACK 7593b06bd1262f438bf34769ecc00e9c22328e56 naumenkogs: ACK 7593b06bd1262f438bf34769ecc00e9c22328e56. laanwj: Code review ACK 7593b06bd1262f438bf34769ecc00e9c22328e56 kristapsk: ACK 7593b06bd1262f438bf34769ecc00e9c22328e56. Code looks correct, tested that functional test suite passes and also that `test/functional/p2p_addrv2_replay.py` fails if I undo changes in `IsRelayable()`. Tree-SHA512: c9feec4a9546cc06bc2fec6d74f999a3c0abd3d15b7c421c21fcf2d610eb94611489e33d61bdcd5a4f42041a6d84aa892f7ae293b0d4f755309a8560b113b735
2021-07-15Merge bitcoin/bitcoin#22447: test: whitelist rpc_rawtransaction peers to ↵fanquake
speed up tests a3d6ec5bb567481a634638cea7ae37c355119a7b test: move rpc_rawtransaction tests to < 30s group (Jon Atack) 5a1ed96077852c739034c21d399da65db09e7714 test: whitelist rpc_rawtransaction peers to speed up tests (Jon Atack) Pull request description: Speed up the somewhat slow `rpc_rawtransaction.py` test by more than 3x (from 45-55 seconds to 15 seconds on a laptop running 2 x 2.5GHz). ACKs for top commit: mjdietzx: ACK a3d6ec5bb567481a634638cea7ae37c355119a7b kristapsk: ACK a3d6ec5bb567481a634638cea7ae37c355119a7b theStack: ACK a3d6ec5bb567481a634638cea7ae37c355119a7b 🐎 brunoerg: tACK a3d6ec5bb567481a634638cea7ae37c355119a7b Tree-SHA512: f1d105594c9b5b257a7096b631a6fa5aeb50e330a351f75c2d6ffa7dd73abdb6e1f596a78c16d204a9bac3fe506e0519f9ad96bb8477ab6424c8e18125ccb659
2021-07-14test: move rpc_rawtransaction tests to < 30s groupJon Atack
2021-07-14test: whitelist rpc_rawtransaction peers to speed up testsJon Atack
in my testing from 45-55 seconds to 15.
2021-07-14test: Fix wallet_listdescriptors.py if bdb is not compiledHennadii Stepanov
2021-07-14Merge bitcoin/bitcoin#20354: test: Add feature_taproot.py --previous_releaseMarcoFalke
fa80e10d94dbf86da84fc761b09fb631155a5b25 test: Add feature_taproot.py --previous_release (MarcoFalke) 85ccffa26686c6c9adbd18bdde37fc1747281bab test: move releases download incantation to README (Sjors Provoost) 29d6b1da2a862bfbb14e7821979c97416c5400e8 test: previous releases: add v0.20.1 (Sjors Provoost) Pull request description: Disabling the new consensus code at runtime is fine, but potentially fragile and incomplete. Fix that by giving the option to run with a version that has been compiled without any taproot code. ACKs for top commit: Sjors: tACK fa80e10 NelsonGaldeman: tACK fa80e10d94dbf86da84fc761b09fb631155a5b25 Tree-SHA512: 1a1feef823f08c05268759645a8974e1b2d39a024258f5e6acecbe25097aae3fa9302c27262978b40f1aa8e7b525b60c0047199010f2a5d6017dd6434b4066f0
2021-07-13Merge bitcoin/bitcoin#22112: Force port 0 in I2PW. J. van der Laan
4101ec9d2e05a35c35f587a28f1feee6cebcc61b doc: mention that we enforce port=0 in I2P (Vasil Dimov) e0a2b390c144e123e2fc8a289fdff36815476964 addrman: reset I2P ports to 0 when loading from disk (Vasil Dimov) 41cda9d075ebcab1dbb950160ebe9d0ba7b5745e test: ensure I2P ports are handled as expected (Vasil Dimov) 4f432bd738c420512a86a51ab3e00323f396b89e net: do not connect to I2P hosts on port!=0 (Vasil Dimov) 1f096f091ebd88efb18154b8894a38122c39624f net: distinguish default port per network (Vasil Dimov) aeac3bce3ead1f24ca782079ef0defa86fd8cb98 net: change I2P seeds' ports to 0 (Vasil Dimov) 38f900290cc3a839e99bef13474d35e1c02e6b0d net: change assumed I2P port to 0 (Vasil Dimov) Pull request description: _This is an alternative to https://github.com/bitcoin/bitcoin/pull/21514, inspired by https://github.com/bitcoin/bitcoin/pull/21514#issuecomment-815049933. They are mutually exclusive. Just one of them should be merged._ Change assumed ports for I2P to 0 (instead of the default 8333) as this is closer to what actually happens underneath with SAM 3.1 (https://github.com/bitcoin/bitcoin/pull/21514#issuecomment-812632520, https://github.com/bitcoin/bitcoin/pull/21514#issuecomment-816564719). Don't connect to I2P peers with advertised port != 0 (we don't specify a port to our SAM 3.1 proxy and it always connects to port = 0). Note, this change: * Keeps I2P addresses with port != 0 in addrman and relays them to others via P2P gossip. There may be non-bitcoin-core-22.0 peers using SAM 3.2 and for them such addresses may be useful. * Silently refuses to connect to I2P hosts with port != 0. This is ok for automatically chosen peers from addrman. Not so ok for peers provided via `-addnode` or `-connect` - a user who specifies `foo.b32.i2p:1234` (non zero port) may wonder why "nothing is happening". Fixes #21389 ACKs for top commit: laanwj: Code review ACK 4101ec9d2e05a35c35f587a28f1feee6cebcc61b jonatack: re-ACK 4101ec9d2e05a35c35f587a28f1feee6cebcc61b per `git range-diff efff9c3 0b0ee03 4101ec9`, built with DDEBUG_ADDRMAN, did fairly extensive testing on mainnet both with and without a peers.dat / -dnsseeds=0 to test boostrapping. Tree-SHA512: 0e3c019e1dc05e54f559275859d3450e0c735596d179e30b66811aad9d5b5fabe3dcc44571e8f7b99f9fe16453eee393d6e153454dd873b9ff14907d4e6354fe
2021-07-12Merge bitcoin/bitcoin#20234: net: don't bind on 0.0.0.0 if binds are ↵W. J. van der Laan
restricted to Tor 2feec3ce3130961f98ceb030951d0e46d3b9096c net: don't bind on 0.0.0.0 if binds are restricted to Tor (Vasil Dimov) Pull request description: The semantic of `-bind` is to restrict the binding only to some address. If not specified, then the user does not care and we bind to `0.0.0.0`. If specified then we should honor the restriction and bind only to the specified address. Before this change, if no `-bind` is given then we would bind to `0.0.0.0:8333` and to `127.0.0.1:8334` (incoming Tor) which is ok - the user does not care to restrict the binding. However, if only `-bind=addr:port=onion` is given (without ordinary `-bind=`) then we would bind to `addr:port` _and_ to `0.0.0.0:8333` in addition. Change the above to not do the additional bind: if only `-bind=addr:port=onion` is given (without ordinary `-bind=`) then bind to `addr:port` (only) and consider incoming connections to that as Tor and do not advertise it. I.e. a Tor-only node. ACKs for top commit: laanwj: Code review ACK 2feec3ce3130961f98ceb030951d0e46d3b9096c jonatack: utACK 2feec3ce3130961f98ceb030951d0e46d3b9096c per `git diff a004833 2feec3c` hebasto: ACK 2feec3ce3130961f98ceb030951d0e46d3b9096c, tested on Linux Mint 20.1 (x86_64): Tree-SHA512: a04483af601706da928958b92dc560f9cfcc78ab0bb9d74414636eed1c6f29ed538ce1fb5a17d41ed82c9c9a45ca94899d0966e7ef93da809c9bcdcdb1d1f040
2021-07-12test: Add functional test for AddrFetch connectionsMartin Zumsande
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2021-07-12net, rpc: Enable AddrFetch connections for functional testingMartin Zumsande
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2021-07-09[test] Allow rebroadcast for same-txid-different-wtxid transactionsglozow
Co-authored-by: John Newbery <john@johnnewbery.com>
2021-07-09[test] Test transactions are not re-added to unbroadcast setDuncan Dean
2021-07-09Merge bitcoin/bitcoin#22253: validation: distinguish between same tx and ↵W. J. van der Laan
same-nonwitness-data tx in mempool b7a8cd9963e810264d3b45d0ad15af863965c47a [test] submit same txid different wtxid as mempool tx (glozow) fdb48163bfbf34f79dc78ffaa2bbf9e39af96687 [validation] distinguish same txid different wtxid in mempool (glozow) Pull request description: On master, if you submit a transaction with the same txid but different witness to the mempool, it thinks the transactions are the same. Users submitting through `BroadcastTransaction()` (i.e. `sendrawtransaction` or the wallet) don't get notified that there's a different transaction in the mempool, although it doesn't crash. Users submitting through `testmempoolaccept()` will get a "txn-already-in-mempool" error. This PR simply distinguishes between `txn-already-in-mempool` and `txn-same-nonwitness-data-in-mempool`, without handling them differently: `sendrawtransaction` still will not throw, but `testmempoolaccept` will give you a different error. I believe the intention of #19645 is to allow full swaps of transactions that have different witnesses but identical nonwitness data. Returning a different error message + adding a test was suggested: https://github.com/bitcoin/bitcoin/pull/19645#issuecomment-705109193 so this is that PR. ACKs for top commit: naumenkogs: ACK b7a8cd9963e810264d3b45d0ad15af863965c47a jnewbery: Code review ACK b7a8cd9963e810264d3b45d0ad15af863965c47a theStack: Code-review ACK b7a8cd9963e810264d3b45d0ad15af863965c47a darosior: re-utACK b7a8cd9963e810264d3b45d0ad15af863965c47a Tree-SHA512: 9c6591edaf8727ba5b4675977adb8cbdef7288584003b6cd659828032dc92d2ae915800a8ef8b6fdffe112c1b660df72297a3dcf2e2e3e1f959c6cb3678c63ee
2021-07-09test: ensure I2P ports are handled as expectedVasil Dimov
2021-07-09Merge bitcoin/bitcoin#22363: test: refactor: use `script_util` helpers for ↵MarcoFalke
creating P2{PKH,SH,WPKH,WSH} scripts 905d672b743edf31531d095ffe800449eaffec69 test: use script_util helpers for creating P2W{PKH,SH} scripts (Sebastian Falbesoner) 285a65ccfde2e811cfe01e916b998c02ee534a97 test: use script_util helpers for creating P2SH scripts (Sebastian Falbesoner) b57b633b942da162045b1fe7743a8abdfeaf60e2 test: use script_util helpers for creating P2PKH scripts (Sebastian Falbesoner) 61b6a017a9f99ef072b2d1980dd547eb20093352 test: wallet util: fix multisig P2SH-P2WSH script creation (Sebastian Falbesoner) Pull request description: PR #18788 (commit 08067aebfd7e838e6ce6b030c31a69422260fc6f) introduced functions to generate output scripts for various types. This PR replaces all manual CScript creations in the P2PKH, P2SH, P2WPKH, P2WSH formats with those helpers in order to increase readability and maintainability over the functional test codebase. The first commit fixes a bug in the wallet_util helper module w.r.t. to P2SH-P2WSH script creation (the result is not used in any test so far, hence it can still be seen as refactoring). The following table shows a summary of the output script patterns tackled in this PR: | Type | master branch | PR branch | | ---------- | ------------- | ------------- | | P2PKH | `CScript([OP_DUP, OP_HASH160, hash160(key), OP_EQUALVERIFY, OP_CHECKSIG])` | `key_to_p2pkh_script(key)` | | | `CScript([OP_DUP, OP_HASH160, keyhash, OP_EQUALVERIFY, OP_CHECKSIG])` | `keyhash_to_p2pkh_script(keyhash)` | | P2SH | `CScript([OP_HASH160, hash160(script), OP_EQUAL])` | `script_to_p2sh_script(script)` | | P2WPKH | `CScript([OP_0, hash160(key)])` | `key_to_p2wpkh_script(key)` | | P2WSH | `CScript([OP_0, sha256(script)])` | `script_to_p2wsh_script(script)` | Note that the `key_to_...` helpers can't be used if an invalid key size (not 33 or 65 bytes) is passed, which is the case in some rare instances where the scripts still have to be created manually. Possible follow-up ideas: * further simplify by identifying P2SH-wrapped scripts and using `key_to_p2sh_p2wpkh_script()` and `script_to_p2sh_p2wsh_script()` helpers * introduce and use `key_to_p2pk_script()` helper for P2PK scripts ACKs for top commit: rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/22363/commits/905d672b743edf31531d095ffe800449eaffec69 LarryRuane: tACK 905d672b743edf31531d095ffe800449eaffec69 0xB10C: ACK 905d672b743edf31531d095ffe800449eaffec69 MarcoFalke: review ACK 905d672b743edf31531d095ffe800449eaffec69 🕹 Tree-SHA512: 7ccfe69699bc81168ac122b03536720013355c1b2fbb088355b616015318644c4d1cd27e20c4f56c89ad083ae609add4bc838cf6316794d0edb0ce9cf7fa0fd8
2021-07-09Merge bitcoin/bitcoin#22381: guix: Test security-check sanity before ↵fanquake
performing them (with macOS) 5b4703c6a70db2fa72fcace56a15db07d4b0acf1 guix: Test security-check sanity before performing them (Carl Dong) 6cf3345297d371b4785d80d54e802b52ff09e8c2 scripts: adjust test-symbol-check for guix release environment (fanquake) 1946b5f77cb5a6bb37500252079c3582cac4a6c9 scripts: more robustly test macOS symbol checks (fanquake) a8127b34bce3597b8091e14057c926197966a234 build: Use and test PE binutils with --reloc-section (Carl Dong) 678348db515c770c4dddfac512cdd97be11d407d guix: Patch binutils to add security-related disable flags (Carl Dong) 9fdc8afe117b7b1ea845f8acae9e831922b8f92b devtools: Improve *-check.py tool detection (Carl Dong) bda62eab38c5dd74e222eddedbca19ace9df6daa ci: skip running the Linux test-security-check target for now (fanquake) d6ef3543ae16847d5a91fa9271acee9bd2164b32 lint: Run mypy with --show-error-codes (Carl Dong) Pull request description: This is #20980 rebased (to include the Boost Process fix), and with an additional commit (892d6897f1e613084aa0517a660eab2412308e6e) to fix running the `test-security-check` target for the macOS build. It should pass inside Guix, as well as when cross-compiling on Ubuntu, or building natively on macOS. Note that the `test-security-check` may output some warnings (similar too): ```bash ld: warning: passed two min versions (10.14, 11.4) for platform macOS. Using 11.4. ld: warning: passed two min versions (10.14, 11.4) for platform macOS. Using 11.4. ld: warning: passed two min versions (10.14, 10.14) for platform macOS. Using 10.14. ``` but those can be ignored, and come about due to us passing `-platform_version` when `-mmacosx-version-min` is already part of `CC`. Guix builds: ```bash 71ed0c7a13a4726300779ffc87f7d271086a2744c36896fe6dc51fe3dc33df2e guix-build-5b4703c6a70d/output/aarch64-linux-gnu/SHA256SUMS.part 9273980a17052c8ec45b77579781c14ab5d189fa25aa29907d5115513dd302b1 guix-build-5b4703c6a70d/output/aarch64-linux-gnu/bitcoin-5b4703c6a70d-aarch64-linux-gnu-debug.tar.gz 9c042179af43c8896eb95a34294df15d4910308dcdba40b2010cd36e192938b8 guix-build-5b4703c6a70d/output/aarch64-linux-gnu/bitcoin-5b4703c6a70d-aarch64-linux-gnu.tar.gz 1ceddecac113f50a952ba6a201cdcdb722e3dc804e663f219bfac8268ce42bf0 guix-build-5b4703c6a70d/output/arm-linux-gnueabihf/SHA256SUMS.part 759597c4e925e75db4a2381c06cda9b9f4e4674c23436148676b31c9be05c7aa guix-build-5b4703c6a70d/output/arm-linux-gnueabihf/bitcoin-5b4703c6a70d-arm-linux-gnueabihf-debug.tar.gz 34e3b6beabaf8c95d7c2ca0d2c3ac4411766694ef43e00bd9783badbbaf045a7 guix-build-5b4703c6a70d/output/arm-linux-gnueabihf/bitcoin-5b4703c6a70d-arm-linux-gnueabihf.tar.gz e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 guix-build-5b4703c6a70d/output/dist-archive/SKIPATTEST.TAG 3664f6ceee7898caa374281fd877a7597fe491fa2e9f0c174c28d889d60b559c guix-build-5b4703c6a70d/output/dist-archive/bitcoin-5b4703c6a70d.tar.gz d6bc35ba0750c1440bb32831b8c12cddee62f6dce10fec2650897444c2bf4748 guix-build-5b4703c6a70d/output/powerpc64-linux-gnu/SHA256SUMS.part a836edf6474ba0c16c19bb217549bac7936c1b44306ed512df58f607ee5568f2 guix-build-5b4703c6a70d/output/powerpc64-linux-gnu/bitcoin-5b4703c6a70d-powerpc64-linux-gnu-debug.tar.gz 7cc91c6805d5069ca3bd1771e77d95f83eb184b137198cbf84d1d11d0a5c5afe guix-build-5b4703c6a70d/output/powerpc64-linux-gnu/bitcoin-5b4703c6a70d-powerpc64-linux-gnu.tar.gz 93b4cb7b83c4975120ad5de5a92f050f5760a2a3f2c37c204c647f5a581c924a guix-build-5b4703c6a70d/output/powerpc64le-linux-gnu/SHA256SUMS.part 2266e2c5d0dafa28c6c057ccfc1c439baeab1d714d8c3f64a83015d2827116d2 guix-build-5b4703c6a70d/output/powerpc64le-linux-gnu/bitcoin-5b4703c6a70d-powerpc64le-linux-gnu-debug.tar.gz 85f41f42c319b83d049d6fd2e2278c07b40a1e28a2eac596427822c0eef9dc3f guix-build-5b4703c6a70d/output/powerpc64le-linux-gnu/bitcoin-5b4703c6a70d-powerpc64le-linux-gnu.tar.gz 1499ca9119926083d8c3714ca10d8d4c8d864cbeee8848fd8445b7a1d081222d guix-build-5b4703c6a70d/output/riscv64-linux-gnu/SHA256SUMS.part 1995fc1a2e45c49d4b0718aff5dcdac931917e8ae9e762fd23f1126abcecc248 guix-build-5b4703c6a70d/output/riscv64-linux-gnu/bitcoin-5b4703c6a70d-riscv64-linux-gnu-debug.tar.gz 266889eb58429a470f0fd7bb123f2ae09b0aef86c47b0390938b3634a8f748a9 guix-build-5b4703c6a70d/output/riscv64-linux-gnu/bitcoin-5b4703c6a70d-riscv64-linux-gnu.tar.gz cdc3a0dcf80b110443dac5ddf8bc951001a776a651c898c5ea49bb2d487bfe29 guix-build-5b4703c6a70d/output/x86_64-apple-darwin18/SHA256SUMS.part 8538d1eab96c97866b24546c453d95822f24cf9c6638b42ba523eb7aa441cb26 guix-build-5b4703c6a70d/output/x86_64-apple-darwin18/bitcoin-5b4703c6a70d-osx-unsigned.dmg d1b73133f1da68586b07292a8425f7f851e93f599c016376f23728c041cf39cc guix-build-5b4703c6a70d/output/x86_64-apple-darwin18/bitcoin-5b4703c6a70d-osx-unsigned.tar.gz 5ad94c5f8a5f29405955ff3ab35d137de1acc04398d6c8298fb187b57a6e316a guix-build-5b4703c6a70d/output/x86_64-apple-darwin18/bitcoin-5b4703c6a70d-osx64.tar.gz 8c6d7b3f847faa7b4d16ceecf228f26f146ea982615c1d7a00c57f9230a0c484 guix-build-5b4703c6a70d/output/x86_64-linux-gnu/SHA256SUMS.part d0a8c99750319ad8046cfa132a54e5c13a08351f94439ae9af0f8e5486c2c2ea guix-build-5b4703c6a70d/output/x86_64-linux-gnu/bitcoin-5b4703c6a70d-x86_64-linux-gnu-debug.tar.gz d816bb26dd4b0e309f2f576b1cccc6d78743fb2f357daad2da09bb1177330971 guix-build-5b4703c6a70d/output/x86_64-linux-gnu/bitcoin-5b4703c6a70d-x86_64-linux-gnu.tar.gz 65caaa7f648c7eab1eb82c3331a2ca25b8cd4fe41439de55604501e02571de55 guix-build-5b4703c6a70d/output/x86_64-w64-mingw32/SHA256SUMS.part 5bf6f7328cbceb0db22a2d7babb07b60cb6dcc19a6db84a1698589b7f5173a06 guix-build-5b4703c6a70d/output/x86_64-w64-mingw32/bitcoin-5b4703c6a70d-win-unsigned.tar.gz 7aabcb56115decef78d3797840b6e49dbc9b202d56f892490e92616fb06fec9e guix-build-5b4703c6a70d/output/x86_64-w64-mingw32/bitcoin-5b4703c6a70d-win64-debug.zip 2f369694648ff9dc5ca1261a1e5874b1c7408ccf2802f9caef56c1334e8a5b7c guix-build-5b4703c6a70d/output/x86_64-w64-mingw32/bitcoin-5b4703c6a70d-win64-setup-unsigned.exe 1c1f92513c4aad38419ff49a7b80bf10e6b1eca01ee8c5e3b2acd1768cf1e3d5 guix-build-5b4703c6a70d/output/x86_64-w64-mingw32/bitcoin-5b4703c6a70d-win64.zip ``` ACKs for top commit: hebasto: Approach ACK 5b4703c6a70db2fa72fcace56a15db07d4b0acf1. Tree-SHA512: 2cd92a245ea64ef7176cf402a1fa5348a9421c30a4d30d01c950c48f6dcc15cf22ce69ffe1657be97e5fccc14bd933d64683c4439b695528ce3dc34d72dda927
2021-07-08Merge bitcoin/bitcoin#22408: test: add tests for `bad-txns-prevout-null` ↵MarcoFalke
reject reason 1f449586a9e39bc4fb53cb5c7a31362e47aea19b test: add `bad-txns-prevout-null` test to mempool_accept.py (Sebastian Falbesoner) aa0a5bb70d77739d43d5a9ceae78fb0c6fafd435 test: add `bad-txns-prevout-null` test case to invalid_txs.py (Sebastian Falbesoner) Pull request description: This simple PR adds missing tests for the reject reason `bad-txns-prevout-null`, which is thrown in the function `CheckTransaction()`: https://github.com/bitcoin/bitcoin/blob/a62fc35a150da584d39d7cd01ade14bbb5002fb9/src/consensus/tx_check.cpp#L52-L54 Basically this condition is met for non-coinbase transactions (the code snippet above only hits if `!tx.IsCoinBase()`) with coinbase-like outpoints, i.e. hash=0, n=0xffffffff. Can be tested by running the functional tests `feature_block.py`, `p2p_invalid_tx.py` and `mempool_accept.py`. Not sure if the redundancy in the tests is desired (I guess it would make sense if the mempool acceptance test also makes use of the invalid_txs templates?). ACKs for top commit: rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/22408/commits/1f449586a9e39bc4fb53cb5c7a31362e47aea19b brunoerg: tACK 1f449586a9e39bc4fb53cb5c7a31362e47aea19b kristapsk: ACK 1f449586a9e39bc4fb53cb5c7a31362e47aea19b, code looks correct and all tests pass. Tree-SHA512: 2d4f940a6ac8e0d80d2670c9e1111cbf43ae6ac62809a2ccf17cffee9a41d387ea4d889ee300eb4a407c055b13bfa5d37102a32ed59964a9b6950bd907ba7204
2021-07-08tree-wide: Rename gitian-keys to builder-keysCarl Dong
2021-07-08[test] submit same txid different wtxid as mempool txglozow
Co-authored-by: Antoine Riard <ariard@student.42.fr> Co-authored-by: Antoine Riard <antoine.riard@gmail.com>
2021-07-07net: don't bind on 0.0.0.0 if binds are restricted to TorVasil Dimov
The semantic of `-bind` is to restrict the binding only to some address. If not specified, then the user does not care and we bind to `0.0.0.0`. If specified then we should honor the restriction and bind only to the specified address. Before this change, if no `-bind` is given then we would bind to `0.0.0.0:8333` and to `127.0.0.1:8334` (incoming Tor) which is ok - the user does not care to restrict the binding. However, if only `-bind=addr:port=onion` is given (without ordinary `-bind=`) then we would bind to `addr:port` _and_ to `0.0.0.0:8333` in addition. Change the above to not do the additional bind: if only `-bind=addr:port=onion` is given (without ordinary `-bind=`) then bind to `addr:port` (only) and consider incoming connections to that as Tor and do not advertise it. I.e. a Tor-only node.
2021-07-07lint: Run mypy with --show-error-codesCarl Dong
When using mypy ignore directives, the error code needs to be specified. Somehow mypy doesn't print it by default...
2021-07-06lint: exclude Guix patches from spell-checkingfanquake
Co-authored-by: Carl Dong <contact@carldong.me>
2021-07-05test: add `bad-txns-prevout-null` test to mempool_accept.pySebastian Falbesoner
2021-07-05test: add `bad-txns-prevout-null` test case to invalid_txs.pySebastian Falbesoner
This reject reason is triggered for non-coinbase transactions with a coinbase-like outpoint, i.e. hash=0, n=0xffffffff. Note that the invalid tx templates are currently used in the functional tests feature_block.py and p2p_invalid_tx.py.
2021-07-05test: use script_util helpers for creating P2W{PKH,SH} scriptsSebastian Falbesoner
2021-07-05test: use script_util helpers for creating P2SH scriptsSebastian Falbesoner
2021-07-01Merge bitcoin/bitcoin#22310: test: Add functional test for replacement relay ↵MarcoFalke
fee check c4ddee64c7f80eee05a95116ef1b1dc8a7601183 test: Add test for replacement relay fee check (Antoine Riard) Pull request description: This PR adds rename the `reject_reason` of our implementation of BIP125 rule 4 and adds missing functional test coverage. Note, `insufficient fee` is already the `reject_reason` of few others `PreChecks` replacement checks and as such might be confusing. > The replacement transaction must also pay for its own bandwidth at or above the rate set by the node's minimum relay fee setting. For example, if the minimum relay fee is 1 satoshi/byte and the replacement transaction is 500 bytes total, then the replacement must pay a fee at least 500 satoshis higher than the sum of the originals. ``` // Finally in addition to paying more fees than the conflicts the // new transaction must pay for its own bandwidth. CAmount nDeltaFees = nModifiedFees - nConflictingFees; if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", hash.ToString(), FormatMoney(nDeltaFees), FormatMoney(::incrementalRelayFee.GetFee(nSize)))); } ``` ACKs for top commit: MarcoFalke: cr ACK c4ddee64c7f80eee05a95116ef1b1dc8a7601183 glozow: ACK c4ddee6, one small suggestion if you retouch. Tree-SHA512: 7c5d1065db6e6fe57a9f083bf051a7a55eb9892de3a2888679d4a6853491608c93b6e35887ef383a9988d14713fa13a0b1d6134b7354af5fd54765f0d4e98568
2021-07-01Merge bitcoin/bitcoin#19651: wallet: importdescriptors update existingfanquake
3efaf83c75cd8dc2fa084537b8ed6715fb58c04d wallet: deactivate descriptor (S3RK) 6737d9655bcf527afbd85d610d805a2d0fd28c4f test: wallet importdescriptors update existing (S3RK) 586f1d53d60880ea2873d860f95e3390016620d1 wallet: maintain SPK consistency on internal flag change (S3RK) f1b7db14748d9ee04735b4968366d33bc89aea23 wallet: don't mute exceptions in importdescriptors (S3RK) bf68ebc1cd555f791103f81adc9111e0e55c8003 wallet: allow to import same descriptor twice (S3RK) Pull request description: Rationale: allow updating existing descriptors with `importdescriptors` command. Currently if you run same `importdescriptors` command twice with a descriptor containing private key you will get very confusing error — `Missing required fields`. What happens is that Wallet tries to write imported private key to the disk, but it exists already so we get `DB_KEYEXIST (-30995)` from BerkelyDB. Please note, that we set `DB_NOOVERWRITE` (I guess not to lose some keys accidentally). The exception is caught in `catch (...)` in rpcdump.cpp with a generic error. With this PR if a descriptor is already present than we will update its activeness, internalness, label, range and next_index. For the range only expansion is allowed (range start can only decrease, range end increase). ACKs for top commit: achow101: re-ACK 3efaf83c75cd8dc2fa084537b8ed6715fb58c04d meshcollider: Code review ACK 3efaf83c75cd8dc2fa084537b8ed6715fb58c04d jonatack: Light ACK 3efaf83c75cd8dc2fa084537b8ed6715fb58c04d per `git range-diff a000cb0 5d96704 3efaf83` and as a sanity check, re-debug-built on debian with gcc 10.2.1 and clang 11, ran wallet_importdescriptors.py Tree-SHA512: 122c4b621d64ec8a3b625f3aed9f01a2b5cbaf2029ad0325b5ff38d67fff5cd35324335fabe2dd5169548b01b267c81be6ae0f5c834342f3d5f6eeed515c4843
2021-06-30test: Add test for replacement relay fee checkAntoine Riard