Using disconnect_p2ps instead of peer_disconnect makes
the node wait for the disconnect to complete. As a result,
we can reuse p2p_idx=0 in the add_outbound_p2p_connection calls.
Currently, debug.log is spammed with messages from random.cpp
when functional tests are run. These logs are not useful for
debugging, and decrease the signal to noise ratio of the logs.
fa54d3011ed0cbb7bcdc76548423ba41f0042832 test: check for false-positives in rpc_scanblocks.py (Sebastian Falbesoner)
3bca6cd61a8cd677023f18c146f2a6534829b1c7 test: add compact block filter (BIP158) helper routines (Sebastian Falbesoner)
25ee74dd11401ce2bfb0b24c4ce70578c5b99e51 test: add SipHash implementation for generic data in Python (Sebastian Falbesoner)
Pull request description:
This PR adds a fixed false-positive element check to the functional test rpc_scanblocks.py by using a pre-calculated scriptPubKey that collides with the regtest genesis block's coinbase output. Note that determining a BIP158 false-positive at runtime would also be possible, but take too long (we'd need to create and check ~800k output scripts on average, which took at least 2 minutes on average on my machine).
The introduced check is related to issue #26322 and more concretely inspired by PR #26325 which introduces an "accurate" mode that filters out these false-positives. The introduced cryptography routines (siphash for generic data) and helpers (BIP158 ranged hash calculation, relevant scriptPubKey per block determination) could potentially also be useful for more tests in the future that involve compact block filters.
ACKs for top commit:
achow101:
ACK fa54d3011ed0cbb7bcdc76548423ba41f0042832
Tree-SHA512: c6af50864146028228d197fb022ba2ff24d1ef48dc7d171bccfb21e62dd50ac80db5fae0c53f5d205edabd48b3493c7aa2040f628a223e68df086ec2243e5a93
dddd1acf58cb7bf328ce3e74d1dc0e8cbd503247 net: Set relay in version msg to peers with relay permission (MacroFake)
Pull request description:
Seems odd to set the `relay` permission in -blocksonly mode and also ask the peer not to relay transactions.
ACKs for top commit:
dergoegge:
ACK dddd1acf58cb7bf328ce3e74d1dc0e8cbd503247
naumenkogs:
ACK dddd1acf58
mzumsande:
ACK dddd1acf58cb7bf328ce3e74d1dc0e8cbd503247
Tree-SHA512: 7bb0e964993ea4982747ae2801fe963ff88586e2ded03015b60ab83172b5b61f2d50e9cde9d7711b7ab207f8639467ecafc4d011ea151ec6c82c722f510f4df7
9e386afb67bf8fa71b72f730da1695eeb11828cd tests: Test that PSBT_OUT_TAP_TREE is included correctly (Andrew Chow)
30ff25cf37eec4b09ab40424eb5d6a4a80410955 psbt: Only include m_tap_tree if it has scripts (Andrew Chow)
0577d423adda8e719d7611d03355680c8fbacab8 psbt: Change m_tap_tree to store just the tuples (Andrew Chow)
22c051ca70bae73e0430b05fb9d879591df27699 tests: Test that PSBT_OUT_TAP_TREE is combined correctly (Andrew Chow)
7df6e1bb77a96eac4fbcba424bbe780636b86650 psbt: Fix merging of m_tap_tree (Andrew Chow)
0652dc53b291bd295caff4093ec2854fd4b34645 [BugFix]: Do not allow deserializing PSBT with empty PSBT_OUT_TAP_TREE (Jeremy Rubin)
Pull request description:
PSBT_OUT_TAP_TREE should not be included for outputs that do not have such a tree. This should be disallowed during parsing, as well as prior to serialization when the field is populated during updating.
Also added some test cases.
Alternative to #25856
ACKs for top commit:
instagibbs:
ACK 9e386afb67
darosior:
ACK 9e386afb67bf8fa71b72f730da1695eeb11828cd
Tree-SHA512: ce5c02a69752d176dbd967c1e8d30129b1905c8f186aeeef034576c1de82059271a1ee846bd040f5be4e66bb77ba711dcf14ac1e597c5707d7e7e2293f6cfefb
3405f3eed5cf841b23a569b64a376c2e5b5026cd test: Test that an unconfirmed not-in-mempool chain is rebroadcast (Andrew Chow)
10d91c5abe9ed7dcc237c9d52c588e7d26e162a4 wallet: Deduplicate Resend and ReacceptWalletTransactions (Andrew Chow)
Pull request description:
Currently `ResendWalletTransactions` (used for normal rebroadcasts) will attempt to rebroadcast all of the transactions in the wallet in the order they are stored in `mapWallet`. This ends up being random as `mapWallet` is a `std::unordered_map`. However `ReacceptWalletTransactions` (used for adding to the mempool on loading) first sorts the txs by wallet insertion order, then submits them. The result is that `ResendWalletTranactions` will fail to rebroadcast child transactions if their txids happen to be lexicographically less than their parent's txid. This PR resolves this issue by combining `ReacceptWalletTransactions` and `ResendWalletTransactions` into a new `ResubmitWalletTransactions` so that the iteration code and basic checks are shared.
A test has also been added that checks that such transaction chains are rebroadcast correctly.
ACKs for top commit:
naumenkogs:
utACK 3405f3eed5cf841b23a569b64a376c2e5b5026cd
1440000bytes:
reACK 3405f3eed5
furszy:
Late code review ACK 3405f3ee
stickies-v:
ACK 3405f3eed5cf841b23a569b64a376c2e5b5026cd
Tree-SHA512: 1240d9690ecc2ae8d476286b79e2386f537a90c41dd2b8b8a5a9c2a917aa3af85d6aee019fbbb05e772985a2b197e2788305586d9d5dac78ccba1ee5aa31d77a
The test checks that parent txs are broadcast before child txs.
The previous behavior is that the rebroadcasting would simply iterate mapWallet. As
mapWallet is a std::unsorted_map, the child can sometimes come before the parent and thus
be rebroadcast in the wrong order and fail the test.
for verbose log messages for development or debugging only, as bitcoind may run
more slowly, that are more granular/frequent than the Debug log level, i.e. for
very high-frequency, low-level messages to be logged distinctly from
higher-level, less-frequent debug logging that could still be usable in production.
An example would be to log higher-level peer events (connection, disconnection,
misbehavior, eviction) as Debug, versus Trace for low-level, high-volume p2p
messages in the BCLog::NET category. This will enable the user to log only the
former without the latter, in order to focus on high-level peer management events.
With respect to the name, "trace" is suggested as the most granular level
in resources like the following:
- https://sematext.com/blog/logging-levels
- https://howtodoinjava.com/log4j2/logging-levels
Update the test framework and add test coverage.
db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 rpc/wallet: add simulaterawtransaction RPC (Karl-Johan Alm)
701a64f548662e01821765b2934b6e4b321fda6d test: add support for Decimal to assert_approx (Karl-Johan Alm)
Pull request description:
(note: this was originally titled "add analyzerawtransaction RPC")
This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally.
I originally proposed this to Elements (https://github.com/ElementsProject/elements/pull/1016) and it was suggested that I propose this upstream.
There is an alternative #22776 to instead add this info to `getbalances` when providing an optional transaction as argument.
ACKs for top commit:
jonatack:
ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849
achow101:
re-ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849
Tree-SHA512: adf222ec7dcdc068d007ae6f465dbc35b692dc7bb2db337be25340ad0c2f9c64cfab4124df23400995c700f41c83c29a2c34812121782c26063b100c7969b89d
Moved `bulk_transaction` into MiniWallet class as `_bulk_tx` private
helper method to be used when the newly added `target_weight` option is
passed to `create_self_transfer*`
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
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.
Rather than abusing the member variables self._priv_key and
self._address to determine the MiniWallet mode, save it explicitly
instead in the constructor to increase the readability and
maintainability of the code.
dcf36fe8e3e1fc1e865072232281b72889586e40 test: implement 'bech32m' mode for `getnewdestination()` helper (Sebastian Falbesoner)
1999dcfa40ddedb6cf15f9d66b90fa0f537b4842 test: add helpers for creating P2TR scripts/addresses from output key (Sebastian Falbesoner)
Pull request description:
This PR adds the missing 'bech32m' mode for the `getnewdestination()` helper and sets it as default, i.e. the function returns a tuple (output x-only-pubkey, scriptPubKey, taproot address) now if not specified otherwise. In a preparation commit, the helpers `output_key_to_p2tr{_script}` are introduced. Note that in contrast to all other common script output types, there are usually _two_ keys involved in creating a taproot output (internal key and output key), hence the prefix `output_` is used to clarify that the output key is expected and the helpers don't do any key tweaking.
Thanks to michaelfolkson (for pointing out this TODO that I forgot about) and sipa (for patiently explaining basic things about BIP341).
ACKs for top commit:
michaelfolkson:
ACK dcf36fe8e3e1fc1e865072232281b72889586e40
w0xlt:
reACK dcf36fe8e3
Tree-SHA512: 5bb8d5fd96c63092ede10c3f022ffb2e13c14e333c4aa73348d95deb70cbf0a74745218dc4a7c419eb846793dd69e8217a7b4332a13ae2b2758e100b51fb1a9f
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.
