aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2022-07-12test: Remove duplicate MAX_BIP125_RBF_SEQUENCE constantMacroFake
2022-07-12scripted-diff: [test] Rename BIP125_SEQUENCE_NUMBER to MAX_BIP125_RBF_SEQUENCEMacroFake
-BEGIN VERIFY SCRIPT- sed -i 's:BIP125_SEQUENCE_NUMBER:MAX_BIP125_RBF_SEQUENCE:g' $(git grep -l BIP125_SEQUENCE_NUMBER ./test) -END VERIFY SCRIPT-
2022-07-12Merge bitcoin/bitcoin#24944: rpc: add getblockfrompeer RPCTypeCheck and ↵MacroFake
invalid input test coverage 2ef5294a5bb68ceb3797d2638567a172cc21699f rpc: add RPCTypeCheck for getblockfrompeer inputs (Jon Atack) 734b9669ff7b2f5e2820993443a6f868f6b0b20a test: add getblockfrompeer coverage of invalid inputs (Jon Atack) Pull request description: The new getblockfrompeer RPC lacks test coverage for invalid arguments, and its error messages are not harmonized with the existing RPCs. Fix all issues. ACKs for top commit: brunoerg: ACK 2ef5294a5bb68ceb3797d2638567a172cc21699f Tree-SHA512: 454782cf6a44fd0e05483bb152153667ef5c8021358385ddcf89724fbbbd35e187362bdff757e00c99319527bc4c0b20c7187f67241d4585d767a29787142f25
2022-07-12Merge bitcoin/bitcoin#25592: test persistence of non-mempool tx prioritisationMacroFake
a9790ba95fa0ad9344ca9a6fda9262ea19ef4649 [test] persist prioritisation of transactions not in mempool (glozow) Pull request description: We persist tx prioritisation/fee deltas in mempool.dat (see `DumpMempool`). It seems we have test coverage for persistence of modified fees of mempool entries (see `vinfo` loop), but not for the prioritisation of transactions not in mempool (see `mapDeltas`). Relevant: https://github.com/bitcoin/bitcoin/pull/25487#discussion_r917490221 ACKs for top commit: darosior: utACK a9790ba95fa0ad9344ca9a6fda9262ea19ef4649 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25592/commits/a9790ba95fa0ad9344ca9a6fda9262ea19ef4649 Tree-SHA512: 3f2769a917041f12414584f69b2239eef57586f9975869e826f356633fcaf893fde15500619b302e7663de14f3661c6cba22c7524cab5286e715e2c105726521
2022-07-12Merge bitcoin/bitcoin#25575: Address comments remaining from #25353glozow
1056bbdfcd40b6bc4e16844c6da5cf666e87b1bc Address comments remaining from #25353 (Antoine Riard) Pull request description: This PR should address the remaining comments from #25353. ACKs for top commit: MarcoFalke: cr ACK 1056bbdfcd40b6bc4e16844c6da5cf666e87b1bc glozow: ACK 1056bbdfcd40b6bc4e16844c6da5cf666e87b1bc w0xlt: cr ACK https://github.com/bitcoin/bitcoin/pull/25575/commits/1056bbdfcd40b6bc4e16844c6da5cf666e87b1bc Tree-SHA512: 194524193b1f087742c04d3cbe221e2ccf62e1f9303dc6668d62b73bd2dc0c039b7d68b33658dbee7809bd14bb8a5479f8e7928180b18c3180fdfbe3876c3ca1
2022-07-12[test] persist prioritisation of transactions not in mempoolglozow
2022-07-11Address comments remaining from #25353Antoine Riard
2022-07-11test: speedup wallet_coinbase_category.pyfurszy
No need to create a chain for it (nor use the cache).
2022-07-11Merge bitcoin/bitcoin#25512: test: remove wallet dependency and refactor ↵MacroFake
rpc_signrawtransaction.py 0ee43d13e993a59fe1ba509f617dd0e01e1068e2 test: refactor rpc_signrawtransaction.py (Ayush Sharma) Pull request description: `rpc_signrawtransaction.py` currently tests the `signrawtransactionwithkey` and `signrawtransactionwithwallet` RPCs. This PR splits `rpc_signrawtransaction.py` into 1. `rpc_signrawtransactionwithkey.py`: the tests for `signrawtransactionwithkey` are moved here and this test can now be run with the wallet disabled. 2. `wallet_signrawtransactionwithwallet.py`: wallet only tests for `signrawtransactionwithwallet.py` ACKs for top commit: aureleoules: tACK 0ee43d13e993a59fe1ba509f617dd0e01e1068e2. Tree-SHA512: c7bd2ea746345c978eae231a781fc52953b9d277eb9f8abb9c3270fe1d9f678f23f3784377d7303a2aa23d7ad90097245e517d386b27b4e0016585dfddcb9d49
2022-07-10test: refactor: pass absolute fee in `create_lots_of_big_transactions` helperSebastian Falbesoner
2022-07-08Merge bitcoin/bitcoin#25337: refactor: encapsulate wallet's address book accessAndrew Chow
d69045e291e32e02d105d1b5ff1c8b86db0ae69e test: add coverage for 'listreceivedbyaddress' no change addrs return (furszy) 324f00a6420bbd64c67c264e50632e6fa36ae732 refactor: 'ListReceived' use optional for filtered address (furszy) b459fc122feace9e9a738c48aab21961cf15dddc refactor: RPC 'ListReceived', encapsulate m_address_book access (furszy) fa9f2ab8fd53075d2a3ec93ddac4908e73525c46 refactor: RPC 'listlabels', encapsulate 'CWallet::ListAddrBookLabels' functionality (furszy) 83e42c4b94e376a19d3eb0a2379769b8b8ac5fc8 refactor: use 'ForEachAddrBookEntry' in RPC 'getaddressesbylabel' (furszy) 2b48642499016cb357e4bcec32481cd50361194e refactor: use ForEachAddrBookEntry in interfaces::getAddresses (furszy) 032842ae4196aaed5ea3567ea01a61ed75ab2edd wallet: implement ForEachAddrBookEntry method (furszy) 09649bc95d5f2855a54a8cf02e65215a3b333c92 refactor: implement general 'ListAddrBookAddresses' for addressbook destinations lookup (furszy) 192eb1e61c3c43baec7f32c498ab0ce0656a58f7 refactor: getAddress don't access m_address_book, use FindAddressEntry function (furszy) Pull request description: ### Context The wallet's `m_address_book` field is being accessed directly from several places across the sources. ### Problem Code structure wise, we shouldn't be accessing it directly. It could end up being modified by mistake (from a place that has nothing to do with the wallet like an RPC command or the GUI) and cause a bigger issue: like an address book entry 'purpose' string change, which if done badly (from 'send' to 'receive'), could end up in a user sharing a "receive" address that he/she doesn't own. ### Solution Encapsulate `m_address_book` access inside the wallet. ------------------------------------------------------- Extra Note: This is the initial step towards decoupling the address book functionality from the wallet's sources. In other words, the creation of the `AddressBookManager` (which will be coming in a follow-up PR). ACKs for top commit: achow101: ACK d69045e291e32e02d105d1b5ff1c8b86db0ae69e theStack: ACK d69045e291e32e02d105d1b5ff1c8b86db0ae69e ✅ w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25337/commits/d69045e291e32e02d105d1b5ff1c8b86db0ae69e Tree-SHA512: dba17acd86f171b4e9af0223bbbcad380048570f6a2f6a92732a51f01abe8806debaf65c9e9e5569fa76a541903cbb50adcb5f56ef77858151c698ae6b218e2a
2022-07-08Merge bitcoin/bitcoin#25353: Add a `-mempoolfullrbf` node settingMacroFake
4c9666bd73645b94ae81be1faf7a0b8237dd6e99 Mention `mempoolfullrbf` in policy/mempool-replacements.md (Antoine Riard) aae66ab43d794bdfaa2dade91760cc55861c9693 Update getmempoolinfo RPC with `mempoolfullrbf` (Antoine Riard) 3e27e317270fdc2dd02794fea9da016387699636 Introduce `mempoolfullrbf` node setting. (Antoine Riard) Pull request description: This is ready for review. Recent discussions among LN devs have brought back on the surface concerns about the security of multi-party funded transactions against pinnings attacks and other mempool-based nuisances. The lack of full-rbf transaction-relay topology connected to miners open the way to cheap and naive DoS against multi-party funded transactions (e.g coinjoins, dual-funded channels, on-chain DLCs, ...) without solutions introducing an overhead cost or centralization vectors afaik . For more details, see [0]. This PR implements a simple `fullrbf` setting, where the node always allows transaction replacement, ignoring BIP125 opt-in flag. The default value of the setting stays **false**, therefore opt-in replacement is still the default Bitcoin Core replacement policy. Contrary to a previous proposal of mine and listening to feedbacks collected since then [1], I think this new setting simply offers more flexibility in a node transaction-relay policy suiting one's application requirements, without arguing a change of the default behavior. I [posted](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-June/020557.html) on the ML to invite operators with a bitcoin application sensitive to full-rbf (e.g dual-funded LN channels service providers) or mempool researchers to join a bootstrapped full-rbf activated peers network for experimentation and learning. If people have strong opinions against the existence of such full-rbf transaction-relay network, I'm proposing to express them on the future thread. [0] https://lists.linuxfoundation.org/pipermail/lightning-dev/2021-May/003033.html [1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-June/019074.html Follow-up suggestions : - soft-enable opt-in RBF in the wallet : https://github.com/bitcoin/bitcoin/pull/25353#issuecomment-1154918789 - p2p discovery and additional outbound connection to full-rbf peers : https://github.com/bitcoin/bitcoin/pull/25353#issuecomment-1156044401 - match the code between RPC, wallet and mempool about disregard of inherited signaling : #22698 ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/25353/commits/4c9666bd73645b94ae81be1faf7a0b8237dd6e99 glozow: ACK 4c9666bd73645b94ae81be1faf7a0b8237dd6e99, a few nits which are non-blocking. w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25353/commits/4c9666bd73645b94ae81be1faf7a0b8237dd6e99 Tree-SHA512: 9e288bf22e06a9808804e58178444ef1830c3fdd42fd8a7cd7ffb101f8f586e08b000679be407d63ca76a56f7216227b368ff630c81f3fac3243db1a1202ab1c
2022-07-07Merge bitcoin/bitcoin#25522: test: Remove -acceptnonstdtxn=1 from feature_rbf.pyMacroFake
fad690ba0a62dfd97ef22711b2344909fcd1fb73 test: Remove -acceptnonstdtxn=1 from feature_rbf.py (MacroFake) fa5059b7dfba6d95c14a12f21d02b63fe29b3f2b test: Make the scriptPubKey of MiniWallet created txs mutable (MacroFake) fa2924582726ee00b392bd0b5591e7d9770e1b90 test: Allow setting sequence per input in MiniWallet create_self_transfer_multi (MacroFake) fac3800d2c962420303ab5c61b2f02f35b9f693a test: Allow amount_per_output in MiniWallet create_self_transfer_multi (MacroFake) 2222842ae73f85494797b14753bc18446e4817a2 test: Allow absolute fee in MiniWallet create_self_transfer (MacroFake) Pull request description: On the main network, nonstandard transactions are hardly relayed, so it seems odd that one of our policy test requires a policy setting opposite of the norm. Surely it is also important to test that nonstandard transactions can be replaced. However, rbf code should not care about the standardness at all. Moreover, I think testing nonstandardness rbf is of lower priority than testing the stuff that actually happens in production. ACKs for top commit: theStack: re-ACK fad690ba0a62dfd97ef22711b2344909fcd1fb73 jamesob: ACK fad690ba0a62dfd97ef22711b2344909fcd1fb73 ([`jamesob/ackr/25522.1.MarcoFalke.test_remove_acceptnonstd`](https://github.com/jamesob/bitcoin/tree/ackr/25522.1.MarcoFalke.test_remove_acceptnonstd)) Tree-SHA512: e0a0c808bccdddf738fb6a84e5e5672d7c341bffd941c4f0c232112bfc68265fa81a2e42ddcab107d586358ffdb3dccc46bb5533d46999fd9ab024169dac0f78
2022-07-07Merge bitcoin/bitcoin#25525: test: remove wallet dependency from ↵MacroFake
mempool_updatefromblock.py eac1099e0071bfe11fe664a8a490eee6bbc80c47 test: remove wallet dependency from mempool_updatefromblock.py (Ayush Sharma) Pull request description: This PR enables one of the non-wallet functional tests(`mempool_updatefromblock.py`) to be run with the wallet disabled. ACKs for top commit: aureleoules: tACK eac1099e0071bfe11fe664a8a490eee6bbc80c47. Tree-SHA512: 9734815f2d2e65e8813bd776cf1d847a55ba4181e218c5e7b066ec69a556261069214f675681d672f5d7b0ba8e06342c4a456619dcc005cbf5618a0527303b7f
2022-07-06Introduce `mempoolfullrbf` node setting.Antoine Riard
This new node policy setting enables to accept replaced-by-fee transaction without inspection of the replaceability signaling as described in BIP125 "explicit signaling". If turns on, the node mempool accepts transaction replacement as described in `policy/mempool-replacements.md`. The default setting value is `false`, implying opt-in RBF is enforced.
2022-07-06test: Remove -acceptnonstdtxn=1 from feature_rbf.pyMacroFake
2022-07-05Merge bitcoin/bitcoin#19393: test: Add more tests for orphan tx handlingMacroFake
c0a5fceee9858afd24fe0bf655b7b30728e96e78 test: Add test for erase orphan tx conflicted by block (Hennadii Stepanov) fa45bb21193ae0c220cfc224d5e3ea0e7f3ec988 test: Add test for erase orphan tx included by block (Hennadii Stepanov) 5c049780c8b310428cf72fb304bf0c1071742785 test: Add test for erase orphan tx from peer (Hennadii Stepanov) Pull request description: This PR adds test coverage for the following cases: - erase orphan transactions when a peer is disconnected - erase an orphan transaction when it is included in a new tip block - erase an orphan transaction when it is conflicted with other transactions included in a new tip block Found useful while working on #19374. ACKs for top commit: aureleoules: tACK c0a5fceee9858afd24fe0bf655b7b30728e96e78 (`make check` and `test/functional/test_runner.py`). kouloumos: ACK c0a5fceee9858afd24fe0bf655b7b30728e96e78 with a nit per https://github.com/bitcoin/bitcoin/pull/19393#discussion_r899156623. pg156: Reviewed to https://github.com/bitcoin/bitcoin/pull/19393/commits/c0a5fceee9858afd24fe0bf655b7b30728e96e78. Concept ACK. Agree due to the lack of RPC calls to inspect orphan pool, using `assert_debug_log` to match strings in log is a reasonable way to test. Tree-SHA512: 98f8deeee2d1c588c7e28a82e513d4a18655084198369db33fe2710458251eeaffed030626940072d7576f57fcbf7d856d761990129e2ca9e372d2ccbd86d07d
2022-07-04Merge bitcoin/bitcoin#25454: p2p: Avoid multiple getheaders messages in ↵fanquake
flight to the same peer 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 Replace GetTime() with NodeClock in MaybeSendGetHeaders() (Suhas Daftuar) abf5d16c24cb08b0451bdbd4d1de63a12930e8f5 Don't send getheaders message when another request is outstanding (Suhas Daftuar) ffe87db247b19ffb8bfba329c5dd0be39ef5a53f Cleanup received_new_header calculation to use WITH_LOCK (Suhas Daftuar) 6d95cd3e7444ebaaabb64a76783ea3551530f1d7 Move peer state updates from headers message into separate function (Suhas Daftuar) 2b341db731793844f12944363186edea23eabdeb Move headers direct fetch to end of ProcessHeadersMessage (Suhas Daftuar) 29c45185223441943ab610e62937a118c7c3a5b2 Move headers-direct-fetch logic into own function (Suhas Daftuar) bf8ea6df75749c27f753b562c4724b3f8d263ad4 Move additional headers fetching to own function (Suhas Daftuar) 9492e93bf9f4a841bf43ca4b593871c0863d5b63 Add helper function for checking header continuity (Suhas Daftuar) 7f2450871b3ea0b4d02d56bd2ca365fcc25cf90e Move handling of unconnecting headers into own function (Suhas Daftuar) Pull request description: Change `getheaders` messages so that we wait up to 2 minutes for a response to a prior `getheaders` message before issuing a new one. Also change the handling of the `getheaders` message sent in response to a block INV, so that we no longer use the hashstop variable (including the hash stop will just mean that if our peer's headers chain is longer, then we won't learn it, so there's no benefit to using hashstop). Also, now respond to a `getheaders` during IBD with an empty headers message (rather than nothing) -- this better conforms to the intent of the new logic that it's better to not ignore a peer's `getheaders` message, even if you have nothing to give. This also avoids a lot of functional tests breaking. This PR also reworks the headers processing logic to make it more readable. ACKs for top commit: ajtowns: ACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 ; code review, check over new logic of when to send getheaders messages dergoegge: Code review ACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 mzumsande: Code Review ACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 sipa: utACK 99f4785cad94657dcf349d00fdd6f1d44cac9bb0 w0xlt: tACK https://github.com/bitcoin/bitcoin/pull/25454/commits/99f4785cad94657dcf349d00fdd6f1d44cac9bb0 Good improvement in the code. Tree-SHA512: b8a63f6f71ac83e292edc0200def7835ad8b06b2955dd34e3ea6fac85980fa6962efd31d689ef5ea121ff5477ec14aafa4bbe2d0db134c05f4a31a57a8ced365
2022-07-03test: pass `dustrelayfee=0` option for tests using dust (instead of ↵Sebastian Falbesoner
`acceptnonstdtxn=1`) By specifying the `dustrelayfee=0` option instead of the more generic `acceptnonstdtxn=1`, we can be more specific about what part of the transaction is non-standard and can be sure that all other aspects follow the standard policy.
2022-07-01test: remove wallet dependency from mempool_updatefromblock.pyAyush Sharma
This functional test can now be run with the wallet disabled.
2022-07-01test: Make the scriptPubKey of MiniWallet created txs mutableMacroFake
This makes individual bytes of the scriptPubKey mutable, previously it could only be re-assigned as a whole.
2022-07-01test: Allow setting sequence per input in MiniWallet create_self_transfer_multiMacroFake
Previously it was only possible to set the same sequence in all inputs
2022-07-01test: Allow amount_per_output in MiniWallet create_self_transfer_multiMacroFake
2022-07-01test: Allow absolute fee in MiniWallet create_self_transferMacroFake
2022-06-30Merge bitcoin/bitcoin#25364: test: remove wallet dependency from ↵MacroFake
feature_nulldummy.py 50ba6697f33b44e475ed65137f7ff0444f6c4ca9 remove unused functions (Ayush Sharma) eec23dad1ec471641dcc74f6679e5c0eda44da94 test: remove wallet dependency from feature_nulldummy.py (Ayush Sharma) Pull request description: This PR enables one of the non-wallet functional tests (`feature_nulldummy.py`) to be run even with the Bitcoin Core wallet disabled. Commit 1: removes wallet dependency and `test_runner.py` is edited to make sure the test only runs once. Commit 2: the functions `create_transaction()` and `create_raw_transaction()` in `blocktools.py` are no longer needed and hence removed. ACKs for top commit: kouloumos: re-ACK 50ba6697f33b44e475ed65137f7ff0444f6c4ca9, all comments have been addressed. Tree-SHA512: 3bc3d2766e53dba3d56a03f2c476442608ac693f51d84f4632a22a2cf169bc02c10bf92b676f7d57acb4f0ad86f307d37ab63f936b44b3585ee3c9d08cd0335f
2022-06-30Merge bitcoin/bitcoin#24836: add RPC (-regtest only) for testing package policyfanquake
e866f0d0666664885d4c15c79bf59cc59975887a [functional test] submitrawpackage RPC (glozow) fa076515b07ac4b10b2134e323bf4f56be5996a8 [rpc] add new submitpackage RPC (glozow) Pull request description: It would be nice for LN/wallet/app devs to test out package policy, package RBF, etc., but the only interface to do so right now is through unit tests. This PR adds a `-regtest` only RPC interface so people can test by submitting raw transaction data. It is regtest-only, as it would be unsafe/confusing to create an actual mainnet interface while package relay doesn't exist. Note that the functional tests are there to ensure the RPC interface is working properly; they aren't for testing policy itself. See src/test/txpackage_tests.cpp. ACKs for top commit: t-bast: Tested ACK against eclair https://github.com/bitcoin/bitcoin/pull/24836/commits/e866f0d0666664885d4c15c79bf59cc59975887a ariard: Code Review ACK e866f0d0 instagibbs: code review ACK e866f0d0666664885d4c15c79bf59cc59975887a Tree-SHA512: 824a26b10d2240e0fd85e5dd25bf499ee3dd9ba8ef4f522533998fcf767ddded9f001f7a005fe3ab07ec95e696448484e26599803e6034ed2733125c8c376c84
2022-06-30Merge bitcoin/bitcoin#25503: test: pass `datacarriersize` option for tests ↵MacroFake
using large outputs (instead of `acceptnonstdtxn`) 475aae846e71e355e8f70c0a1857339b1124bcc0 test: pass `datacarriersize` option for tests using large outputs (instead of `acceptnonstdtxn`) (Sebastian Falbesoner) b1ba3ed155e89fb6e30237cba8bc8028c7906830 test: let `gen_return_txouts` create a single large OP_RETURN output (Sebastian Falbesoner) f319287d819cc97b5422248aacf447ca190e20f6 test: assert serialized txouts size of `gen_return_txouts` helper (Sebastian Falbesoner) Pull request description: By specifying the `datacarriersize` option instead of the more generic `acceptnonstdtxn` for functional tests, we can be more specific about what part of the transaction is non-standard and can be sure that all other aspects follow the standard policy. Transactions with more than one OP_RETURN output are [never considered standard](https://github.com/bitcoin/bitcoin/blob/749b80b29e875cc6afa1c2674cccdfd7115cc16a/src/policy/policy.cpp#L149-L153), i.e. we have to change the `gen_return_txouts` helper to create only a single output in order to get rid of the `acceptnonstdxtn` option. Note that on master there is currently no test using the `datacarriersize` parameter, so this PR indirectly also increases the test coverage. The change affects the tests `mempool_limit.py`, `mining_prioritisetransaction.py` (call `gen_return_txouts` directly) and `feature_maxuploadtarget.py` (calls `gen_return_txouts` indirectly via the `mine_large_block(...)` helper). Top commit has no ACKs. Tree-SHA512: c17f032e00d28f5e5880a4d378773fbc8b1995ea9c377f237598d412628fe117f497a44ebdfa8af8cd8a3b1e3127e0cf7692efbf5c833c713764a71a85301f23
2022-06-30test: refactor rpc_signrawtransaction.pyAyush Sharma
rpc_signrawtransaction.py is split into rpc_signrawtransactionwithkey.py and wallet_signrawtransactionwithwallet.py. rpc_signrawtransactionwithkey.py can be run with the wallet disabled.
2022-06-30test: passing a non-positive integer value to `-peertimeout` should throw an ↵brunoerg
error
2022-06-29test: pass `datacarriersize` option for tests using large outputs (instead ↵Sebastian Falbesoner
of `acceptnonstdtxn`) By specifying the `datacarriersize` option instead of the more generic `acceptnonstdtxn`, we can be more specific about what part of the transaction is non-standard and can be sure that all other aspects follow the standard policy.
2022-06-29test: let `gen_return_txouts` create a single large OP_RETURN outputSebastian Falbesoner
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.
2022-06-29test: assert serialized txouts size of `gen_return_txouts` helperSebastian Falbesoner
This assures that changing the internals of the helper function still leads to the expected outcome sizewise (preparation for the next commit).
2022-06-29Merge bitcoin/bitcoin#25290: [kernel 3a/n] Decouple `CTxMemPool` from ↵MacroFake
`ArgsManager` d1684beabe5b738c2cc83de83e1aaef11a761b69 fees: Pass in a filepath instead of referencing gArgs (Carl Dong) 9a3d825c30e8e6118d74a4e568744cb9d03f7f5d init: Remove redundant -*mempool*, -limit* queries (Carl Dong) 6c5c60c4124293d948735756f84efc85262ea66f mempool: Use m_limit for UpdateTransactionsFromBlock (Carl Dong) 9e93b1030182eff92ef91181e17c7dd498c7e164 node/ifaces: Use existing MemPoolLimits (Carl Dong) 38af2bcf358a72b9457d370282e57f4be1c5c849 mempoolaccept: Use limits from mempool in constructor (Carl Dong) 9333427014695ac235c96d48791098168dfdc9db mempool: Introduce (still-unused) MemPoolLimits (Carl Dong) 716bb5fbd31077bbe99d11a54d6c2c250afc8085 scripted-diff: Rename anc/desc size limit vars to indicate SI unit (Carl Dong) 1ecc77321deb61b9f6888e4e10752b9d972fd26e scripted-diff: Rename DEFAULT_MEMPOOL_EXPIRY to indicate time unit (Carl Dong) aa9141cd8185cb7ad532bc16feb9d302b05d9697 mempool: Pass in -mempoolexpiry instead of referencing gArgs (Carl Dong) 51c7a41a5eb6fcb60333812c770d80227cf7b64d init: Only determine maxmempool once (Carl Dong) 386c9472c8764738282e6d163b42e15a8feda7ea mempool: Make GetMinFee() with custom size protected (Carl Dong) 82f00de7a6a60cbc9ad0c6e1d0ffb1bc70c49af5 mempool: Pass in -maxmempool instead of referencing gArgs (Carl Dong) f1941e8bfd2eecc478c7660434b1ebf6a64095a0 pool: Add and use MemPoolOptions, ApplyArgsManOptions (Carl Dong) 0199bd35bb44e32ee0db9b51c9d1bd7518c26f19 fuzz/rbf: Add missing TestingSetup (Carl Dong) ccbaf546a68d6cda8ed3efd0598c0e4121b366bb scripted-diff: Rename DEFAULT_MAX_MEMPOOL_SIZE to indicate SI unit (Carl Dong) fc02f77ca604f0221171bfde3059b34f5d0fb1cd ArgsMan: Add Get*Arg functions returning optional (Carl Dong) Pull request description: This is part of the `libbitcoinkernel` project: #24303, https://github.com/bitcoin/bitcoin/projects/18 ----- As mentioned in the Stage 1 Step 2 description of [the `libbitcoinkernel` project](https://github.com/bitcoin/bitcoin/issues/24303), `ArgsManager` will not be part of `libbitcoinkernel`. Therefore, it is important that we remove any dependence on `ArgsManager` by code that will be part of `libbitcoinkernel`. This is the first in a series of PRs aiming to achieve this. This PR removes `CTxMemPool+MempoolAccept`'s dependency on `ArgsManager` by introducing a `CTxMemPool::Options` struct, which is used to specify `CTxMemPool`'s various options at construction time. These options are: - `-maxmempool` -> `CTxMemPool::Options::max_size` - `-mempoolexpiry` -> `CTxMemPool::Options::expiry` - `-limitancestorcount` -> `CTxMemPool::Options::limits::ancestor_count` - `-limitancestorsize` -> `CTxMemPool::Options::limits::ancestor_size` - `-limitdescendantcount` -> `CTxMemPool::Options::limits::descendant_count` - `-limitdescendantsize` -> `CTxMemPool::Options::limits::descendant_size` More context can be gleaned from the commit messages. The important commits are: - 56eb479ded8bfb2ef635bb6f3b484f9d5952c70d "pool: Add and use MemPoolOptions, ApplyArgsManOptions" - a1e08b70f3068f4e8def1c630d8f50cd54da7832 "mempool: Pass in -maxmempool instead of referencing gArgs" - 6f4bf3ede5812b374828f08fc728ceded2f10024 "mempool: Pass in -mempoolexpiry instead of referencing gArgs" - 5958a7fe4806599fc620ee8c1a881ca10fa2dd16 "mempool: Introduce (still-unused) MemPoolLimits" Reviewers: Help needed in the following commits (see commit messages): - a1e08b70f3068f4e8def1c630d8f50cd54da7832 "mempool: Pass in -maxmempool instead of referencing gArgs" - 0695081a797e9a5d7787b78b0f8289dafcc6bff7 "node/ifaces: Use existing MemPoolLimits" Note to Reviewers: There are perhaps an infinite number of ways to architect `CTxMemPool::Options`, the current one tries to keep it simple, usable, and flexible. I hope we don't spend too much time arguing over the design here since that's not the point. In the case that you're 100% certain that a different design is strictly better than this one in every regard, please show us a fully-implemented branch. ----- TODO: - [x] Use the more ergonomic `CTxMemPool::Options` where appropriate - [x] Doxygen comments for `ApplyArgsManOptions`, `MemPoolOptions` ----- Questions for Reviewers: 1. Should we use `std::chrono::seconds` for `CTxMemPool::Options::expiry` and `CTxMemPool::m_expiry` instead of an `int64_t`? Something else? (`std::chrono::hours`?) 2. Should I merge `CTxMemPool::Limits` inside `CTxMemPool::Options`? ACKs for top commit: MarcoFalke: ACK d1684beabe5b738c2cc83de83e1aaef11a761b69 🍜 ryanofsky: Code review ACK d1684beabe5b738c2cc83de83e1aaef11a761b69. Just minor cleanups since last review, mostly switching to brace initialization Tree-SHA512: 2c138e52d69f61c263f1c3648f01c801338a8f576762c815f478ef5148b8b2f51e91ded5c1be915e678c0b14f6cfba894b82afec58d999d39a7bb7c914736e0b
2022-06-28Don't send getheaders message when another request is outstandingSuhas Daftuar
Change getheaders messages so that we wait up to 2 minutes for a response to a prior getheaders message before issuing a new one. Also change the handling of the getheaders message sent in response to a block INV, so that we no longer use the hashstop variable (including the hash stop will just mean that if our peer's headers chain is longer, then we won't learn it, so there's no benefit to using hashstop). Also, now respond to a getheaders during IBD with an empty headers message (rather than nothing) -- this better conforms to the intent of the new logic that it's better to not ignore a peer's getheaders message, even if you have nothing to give. This also avoids a lot of functional tests breaking. p2p_segwit.py is modified to use this same strategy, as the test logic (of expecting a getheaders after a block inv) would otherwise be broken.
2022-06-28scripted-diff: Rename DEFAULT_MEMPOOL_EXPIRY to indicate time unitCarl Dong
Better to be explicit when it comes to time to avoid unintentional bugs. -BEGIN VERIFY SCRIPT- find_regex="DEFAULT_MEMPOOL_EXPIRY" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@\0_HOURS@g" -END VERIFY SCRIPT-
2022-06-28Merge bitcoin/bitcoin#22558: psbt: Taproot fields for PSBTlaanwj
b80de4c505bf6377f2e476133dce6f2a803f1fa1 test: Test signing psbts without explicitly having scripts (Andrew Chow) a73b56888a1562d9fe46b7b1d2eea08802d98dfe wallet: also search taproot pubkeys in FillPSBT (Andrew Chow) 6cff82722f47b589a6a2cb264bfce20f4d45426a sign: Use sigdata taproot spenddata when signing (Andrew Chow) 5f12fe3f36bc8a9ad2733986d9493354265a525c psbt: Implement merge for Taproot fields (Andrew Chow) 1ece9a371510d887ed9612f2d219f8dfae278658 psbt, test: Check for taproot fields in taproot psbt test (Andrew Chow) 496a1bbe5e442ffc0948f626cca1b85a46ef58db taproot: Use pre-existing signatures if available (Andrew Chow) 0ad21e7c558da47f50d6b39974d0d2713e829d25 tests: Test taproot fields for PSBT (Andrew Chow) 103c6fd2791f7e73eeab7f3900fbedd5b550211d psbt: Remove non_witness_utxo for segwit v1+ (Andrew Chow) 7dccdd3157a87f55f5398316b98f909d6d6f1feb Implement decodepsbt for Taproot fields (Andrew Chow) ac7747585fb0629be502a089c9c9be876bd7107d Fill PSBT Taproot output data to/from SignatureData (Andrew Chow) 25b6ae46e7249a1b363ef4fb12375f368903c58e Assert that TaprootBuilder is Finalized during GetSpendData (Andrew Chow) 3ae5b6af21cf45b3da5e341e84f50e0717eaf589 Store TaprootBuilder in SigningProviders instead of TaprootSpendData (Andrew Chow) 4d1223e5123e60be93b5ad42ba0aee72d0612ea7 Fetch key origins for Taproot keys (Andrew Chow) 52e3f2f88ef1ac7062e905bf2d745b70463ee3e9 Fill PSBT Taproot input data to/from SignatureData (Andrew Chow) 05e2cc9a302ba7f14fc65ba255594c047cb44559 Implement de/ser of PSBT's Taproot fields (Andrew Chow) d557eff2add151781537978e27d6f1aff1b83ef7 Add serialization methods to XOnlyPubKey (Andrew Chow) d43923c38155fdadad3837d79c19a84c9d2d7f50 Add TaprootBuilder::GetTreeTuples (Andrew Chow) ce911204e42b8653cad791d1727aa625de9d0079 Move individual KeyOriginInfo de/ser to separate function (Andrew Chow) Pull request description: Implements the Taproot fields for PSBT described in [BIP 371](https://github.com/bitcoin/bips/blob/master/bip-0371.mediawiki). ACKs for top commit: laanwj: Code review ACK b80de4c505bf6377f2e476133dce6f2a803f1fa1 Tree-SHA512: 50b79bb44f353c9ec2ef4c98aac08a81eba560987e5264a5684caa370e9c4e7a8255c06747fc47749511be45b32d01492e015f92b82be8d22bc8bf192073bd26
2022-06-27test: Test signing psbts without explicitly having scriptsAndrew Chow
2022-06-27psbt, test: Check for taproot fields in taproot psbt testAndrew Chow
2022-06-27taproot: Use pre-existing signatures if availableAndrew Chow
Actually use pre-existing signatures in CreateTaprootScriptSig if a signature is found for the given key and leaf hash.
2022-06-27tests: Test taproot fields for PSBTAndrew Chow
2022-06-27test: fix failing test interface_usdt_utxocache.pySebastian Falbesoner
The `from_node` argument doesn't exist anymore for `MiniWallet.create_self_transfer` since PR #25435 (commit fa8421bc5bcd483a9501257073db17ff2e76eb46).
2022-06-27rpc: add RPCTypeCheck for getblockfrompeer inputsJon Atack
2022-06-27test: Remove unused call to generate in rpc_mempool_infoMacroFake
There are already enough blocks
2022-06-27test: Sync MiniWallet utxo state after each generate callMacroFake
2022-06-27test: Drop spent utxos in MiniWallet scan_txMacroFake
2022-06-27test: Return new_utxos from create_self_transfer_multi in MiniWalletMacroFake
2022-06-27test: Return new_utxo from create_self_transfer in MiniWalletMacroFake
2022-06-27Merge bitcoin/bitcoin#23418: Fix signed integer overflow in ↵MacroFake
prioritisetransaction RPC fa07f84e316171d60dd9941fb8db37e0a0de6654 Fix signed integer overflow in prioritisetransaction RPC (MarcoFalke) fa52cf8e11b3af6e0a302d5d17aab6cea78899d5 refactor: Replace feeDelta by m_modified_fee (MarcoFalke) Pull request description: Signed integer overflow is UB in theory, but not in practice. Still, it would be nice to avoid this UB to allow Bitcoin Core to be compiled with sanitizers such as `-ftrapv` or ubsan. It is impossible to predict when and if an overflow occurs, since the overflow caused by a prioritisetransaction RPC might only be later hit when descendant txs are added to the mempool. Since it is impossible to predict reliably, leave it up to the user to use the RPC endpoint responsibly, considering their mempool limits and usage patterns. Fixes: #20626 Fixes: #20383 Fixes: #19278 Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34146 / https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=47132 ## Steps to reproduce Build the code without the changes in this pull. Make sure to pass the sanitizer flag: ``` ./autogen.sh && ./configure --with-sanitizers=signed-integer-overflow && make clean && make -j $(nproc) ``` ### Reproduce on RPC ``` ./src/bitcoind -chain=regtest -noprinttoconsole & ./src/bitcoin-cli -chain=regtest prioritisetransaction 00000000deadbeef00000000deadbeef00000000deadbeef00000000deadbeef 0 9123456789123456789 ./src/bitcoin-cli -chain=regtest prioritisetransaction 00000000deadbeef00000000deadbeef00000000deadbeef00000000deadbeef 0 9123456789123456789 |> txmempool.cpp:920:15: runtime error: signed integer overflow: 9123456789123456789 + 9123456789123456789 cannot be represented in type 'long int' ./src/bitcoin-cli -chain=regtest stop ``` ### By fuzzing ``` wget https://github.com/bitcoin/bitcoin/files/8921302/clusterfuzz-testcase-minimized-validation_load_mempool-5599531390074880.bin.txt FUZZ=validation_load_mempool ./src/test/fuzz/fuzz ./clusterfuzz-testcase-minimized-validation_load_mempool-5599531390074880.bin.txt |> txmempool.cpp:920:15: runtime error: signed integer overflow: 7214801925397553184 + 2314885530818453536 cannot be represented in type 'long int' |> validation_load_mempool: succeeded against 1 files in 0s. ACKs for top commit: vasild: ACK fa07f84e316171d60dd9941fb8db37e0a0de6654 dunxen: ACK fa07f84 LarryRuane: ACK fa07f84e316171d60dd9941fb8db37e0a0de6654 Tree-SHA512: 4a357950af55a49c9113da0a50c2e743c5b752f0514dd8d16cd92bfde2f77dd0ef56aa98452626df6f7f7a5b51d1227021f6bc94091201a179f0d488ee32a0df
2022-06-27Merge bitcoin/bitcoin#25439: rpc: Return incrementalrelayfee in getmempoolinfoMacroFake
fafee78188c3de5f6245ec769429822ca4b98c63 rpc: Return incrementalrelayfee in getmempoolinfo (MacroFake) Pull request description: Seems odd to return other policy info, but not the incremental relay fee ACKs for top commit: 1440000bytes: ACK https://github.com/bitcoin/bitcoin/pull/25439/commits/fafee78188c3de5f6245ec769429822ca4b98c63 w0xlt: Code Review ACK https://github.com/bitcoin/bitcoin/pull/25439/commits/fafee78188c3de5f6245ec769429822ca4b98c63 jarolrod: tACK fafee78188c3de5f6245ec769429822ca4b98c63 Tree-SHA512: faad0af6c039b8257acbeac913bc5dcdb2ea2db304c95e52601536c8de60eb1186e9fbb4a64a68adf476605f18022aeda16a5644a0d7912592b0977e4c029638
2022-06-27Merge bitcoin/bitcoin#25476: test: Remove unnecessary mining from ↵MacroFake
importdescriptors test e3d8d727031c93a68ef2e5e85c83f4718e4c209f test: Remove unnecessary block mining from importdescriptors test (Fabian Jahr) Pull request description: This removes generation of 6 blocks and replaces is with a `sync_all` in the `importdescriptors` test. The generated blocks themself don't seem to serve any purpose in the test. Instead they could make the test flaky (although I did not find open issues pointing to this happening in practice in the CI). Right before the blocks being generated a transaction is created (L454) and later in the test this tx is assumed to be still in the mempool. If the nodes were to sync their mempools before the blocks are generated, the test fails. It currently only seems to work because one node sends the tx while the other generates the blocks and the mempools are not synced fast enough. The `sync_all` is still needed to let nodes catch up at that point. Otherwise races happen further below which the generate call seems to have prevented so far. ACKs for top commit: laanwj: Code review ACK e3d8d727031c93a68ef2e5e85c83f4718e4c209f Tree-SHA512: 14f3dc2938d779d1ad43e09a7e046523fc3c92f41df012833f279a2e88e74c2fcab301fe4f3fcc038bd8460ea1360725a8d1eb5b59acd1039495bacb484fd790
generated by cgit v1.2.3 (git 2.26.2) at 2024-07-17 06:18:40 +0000