77ff0ec1f1 contrib: support reading XORed blocks in linearize-data.py script (Sebastian Falbesoner)
Pull request description:
This PR is a small follow-up for #28052, adding support for the block linearization script to handle XORed blocksdir *.dat files. Note that if no xor.dat file exists, the XOR pattern is set to all-zeros, in order to still support blockdirs that have been created with versions earlier than 28.x.
Partly fixes issue #30599.
ACKs for top commit:
achow101:
ACK 77ff0ec1f1
tdb3:
ACK 77ff0ec1f1
hodlinator:
ACK 77ff0ec1f1
Tree-SHA512: 011eb02e2411de373cbbf4b26db4640fc693a20be8c2430529fba6e36a3a3abfdfdc3b005d330f9ec2846bfad9bfbf34231c574ba99289ef37dd51a68e6e7f3d
6b2dcba076 wallet: List sqlite wallets with empty string name (Ava Chow)
3ddbdd1815 wallet: Ignore .bak files when listing wallet files (Ava Chow)
Pull request description:
When the default wallet is migrated, we do not rename the wallet so we end up having a descriptor wallet with the empty string as its name and the wallet.dat file in the root of the walletdir. This is supposed to be an unsupported configuration and there is no other way to achieve this (other than file copying), but the wallet loading code does not disallow loading such wallets. However `listwalletdir` does not currently list the default wallet if it is sqlite. This is confusing to users, so change `listwalletdir` to include these wallets.
Additionally, the migration of the default wallet, and of any plain wallet files in the walletdir, produces a backup file in the walletdir itself. Since these backups are a BDB file, `listwalletdir` will detect them as being another wallet that we could open, but this is erroneous and could lead to confusion and potentially funds loss if both the backup and the migrated wallet are in use simultaneously. To reduce the likelihood of this issue, don't list these wallets in `listwalletdir`.
***
Possibly we could have more stringent checks on loading to resolve these issues, but I'm concerned that that will just confuse users and gratuitously break things that already worked.
Since the original intent was to disallow default wallets for sqlite/descriptors, a possible alternative would be to prevent people from loading such wallets and change migration to rename those wallets. However, given that this behavior with migrating default wallets has existed since default wallet migration was fixed, I think that making such a change would be confusing and break things for no good reason. Although perhaps we should still do the renaming.
For the backups, we could also change loading to refuse to load any wallet named with `.bak` (or `.legacy.bak`) as such wallets can still be loaded by giving the path to them directly, which some users may do to "restore" the backup. However restricting what can be loaded based on filename seems a little heavyhanded. It wouldn't be funds loss though since the correct way to restore the backup is with `restorewallet`.
ACKs for top commit:
fjahr:
Code review ACK 6b2dcba076
furszy:
Code ACK 6b2dcba076
glozow:
ACK 6b2dcba076
Tree-SHA512: 0b033f6ed55830f8a054afea3fb2cf1fa82a94040053ebfaf123bda36c99f45d3f01a2aec4ed02fed9c61bb3d320b047ed892d7f6644b5a356a7bc5974b10cff
00618e8745 assumeutxo: Drop block height from metadata (Fabian Jahr)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/30514 which has more context and shows how the issue can be reproduced. Since the value in question is removed, there is no test to add to reproduce anything.
This is an alternative approach to #30516 with much of the [code being suggested there](https://github.com/bitcoin/bitcoin/pull/30516#discussion_r1689146902).
ACKs for top commit:
maflcko:
re-ACK 00618e8745🎌
achow101:
ACK 00618e8745
theStack:
Code-review ACK 00618e8745
ismaelsadeeq:
Re-ACK 00618e8745
mzumsande:
ACK 00618e8745
Tree-SHA512: db9575247bae838ad7742a27a216faaf55bb11e022f9afdd05752bb09bbf9614717d0ad64304ff5722a16bf41d8dea888af544e4ae26dcaa528c1add0269a4a8
Although it is not explicitly possible to create a default wallet with
descriptors, it is possible to migrate a default wallet and have it end
up being a default wallet with descriptors. These wallets should be
listed by ListDatabases so that it appears in wallet directory listings
to avoid user confusion.
Migration creates backup files in the wallet directory with .bak as the
extension. This pollutes the output of listwalletdir with backup files
that most users should not need to care about.
6bfa26048d testnet: Add timewarp attack prevention for Testnet4 (Fabian Jahr)
0100907ca1 testnet: Add Testnet4 difficulty adjustment rules fix (Fabian Jahr)
74a04f9e7a testnet: Introduce Testnet4 (Fabian Jahr)
Pull request description:
To supplement the [ongoing conceptual discussion about a testnet reset](https://groups.google.com/g/bitcoindev/c/9bL00vRj7OU/m/9yCPo3uUBwAJ) I have drafted a move to v4 including a fix to the difficulty adjustment mechanism, which was part of the motivation that started the discussion.
Conceptual considerations:
- The conceptual discussion about doing a testnet4 or softforking the fix into testnet3 is outside of the scope of this PR and I would ask reviewers to contribute their opinions on this on the ML instead. However, I am happy to adapt this PR to a softfork change on testnet3 if there is consensus for that instead.
- The difficulty adjustment fix suggested here touches the `CalculateNextWorkRequired` function and uses the same logic used in `GetNextWorkRequired` to find the last previous block that was not mined with difficulty 1 under the exceptionf. An alternative fix briefly mentioned on the mailing list by Jameson Lopp would be to "restrict the special testnet minimum difficulty rule so that it can't be triggered on the block right before a difficulty retarget". That would also fix the issue but I find my suggestion here a bit more elegant.
ACKs for top commit:
jsarenik:
tACK 6bfa26048d
achow101:
ACK 6bfa26048d
murchandamus:
tACK 6bfa26048d
Tree-SHA512: 0b8b69a621406a944da5be551b863d065358ba94d85dd3b80d83c412660e230ee93b27316081fbee9b4851cc4ff8585db64c7dfa26cb5148ac835663f2712c3d
e9de0a76b9 doc: release note for 30212 (willcl-ark)
87b1880525 rpc: clarify ALREADY_IN_CHAIN rpc errors (willcl-ark)
Pull request description:
Closes: #19363
Renaming this error improves clarity around the returned error both internally and externally when a transactions' outputs are already found in the utxo set (`TransactionError::ALREADY_IN_CHAIN -> TransactionError::ALREADY_IN_UTXO_SET`)
ACKs for top commit:
tdb3:
ACK e9de0a76b9
ismaelsadeeq:
ACK e9de0a76b9
ryanofsky:
Code review ACK e9de0a76b9.
Tree-SHA512: 7d2617200909790340951fe56a241448f9ce511900777cb2a712e8b9c0778a27d1f912b460f82335844224f1abb4322bc898ca076440959edade55c082a09237
fa895c7283 mingw: Document mode wbx workaround (MarcoFalke)
fa359255fe Add -blocksxor boolean option (MarcoFalke)
fa7f7ac040 Return XOR AutoFile from BlockManager::Open*File() (MarcoFalke)
Pull request description:
Currently the *.dat files in the blocksdir store the data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan them and move them into quarantine, or delete them, or corrupt them. This may cause Bitcoin Core to fail a reorg, or fail to reply to block requests (via P2P, RPC, REST, ...).
Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a random XOR pattern over the dat files when writing or reading them.
Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat files. Any program that intentionally wants to mess with the dat files can still trivially do so.
The XOR pattern is only applied when the blocksdir is freshly created, and there is an option to disable it (on creation), so that people can disable it, if needed.
ACKs for top commit:
achow101:
ACK fa895c7283
TheCharlatan:
Re-ACK fa895c7283
hodlinator:
ACK fa895c7283
Tree-SHA512: c92a6a717da83bc33a9b8671a779eeefde2c63b192362ba1d71e6535ee31d08e2802b74acc908345197de9daac6930e4771595ee25b09acd5a67f7ea34854720
fa530ec543 rpc: Return precise loadtxoutset error messages (MarcoFalke)
faa5c86dbf refactor: Use untranslated error message in ActivateSnapshot (MarcoFalke)
Pull request description:
The error messages should never happen in normal operation. However, if
they do, they are helpful to return to the user to debug the issue. For
example, to notice a truncated file.
This fixes https://github.com/bitcoin/bitcoin/issues/28621
Also includes a minor refactor commit.
ACKs for top commit:
fjahr:
Code review ACK fa530ec543
ryanofsky:
Code review ACK fa530ec543, just adjusting error messages a little since last review. (Thanks!)
Tree-SHA512: 224968c9b13d082ca2ed1f6a8fcc5f51ff16d6c96bd38c3679699505b54337b99cccaf7a8474391f6b11f9ccb101977b4e626898c1217eae95802e290cf105f1
When using `sendrawtransaction` the ALREADY_IN_CHAIN error help string
may be confusing.
Rename TransactionError::ALREADY_IN_CHAIN to
TransactionError::ALREADY_IN_UTXO_SET and update the rpc help string.
Remove backwards compatibility alias as no longer required.
fa3ea3b83c test: Fix intermittent issue in p2p_v2_misbehaving.py (MarcoFalke)
55555574d1 net: Log accepted connection after m_nodes.push_back (MarcoFalke)
Pull request description:
Fix the two issues reported in https://github.com/bitcoin/bitcoin/pull/30468/files#r1688444784:
* Delay a debug log line for consistency.
* Fix an intermittent test issue.
They are completely separate fixes, but both `net` related.
ACKs for top commit:
0xB10C:
Code Review ACK fa3ea3b83c
stratospher:
tested ACK fa3ea3b.
Tree-SHA512: cd6b6e164b317058a305a5c3e38c56c9a814a7469039e1143f1d7addfbc91b0a28506873356b373d97448b46cb6fbe94a1309df82e34c855540b241a09489e8b
75648cea5a test: add P2A ProduceSignature coverage (Greg Sanders)
7998ce6b20 Add release note for P2A output feature (Greg Sanders)
71c9b02a04 test: add P2A coverage for decodescript (Greg Sanders)
1349e9ec15 test: Add anchor mempool acceptance test (Greg Sanders)
9d89209937 policy: stop 3rd party wtxid malleability of anchor spend (Greg Sanders)
b60aaf8b23 policy: make anchor spend standard (Greg Sanders)
455fca86cf policy: Add OP_1 <0x4e73> as a standard output type (Greg Sanders)
Pull request description:
This is a sub-feature taken out of the original proposal for ephemeral anchors #30239
This PR makes *spending* of `OP_1 <0x4e73>` (i.e. `bc1pfeessrawgf`) standard. Creation of this output type is already standard.
Any future witness output types are considered relay-standard to create, but not to spend. This preserves upgrade hooks, such as a completely new output type for a softfork such as BIP341. It also gives us a bit of room to use a new output type for policy uses.
This particular sized witness program has no other known use-cases (https://bitcoin.stackexchange.com/a/110664/17078), s it affords insufficient cryptographic security for a secure commitment to data, such as a script or a public key. This makes this type of output "keyless", or unauthenticated.
As a witness program, the `scriptSig` of the input MUST be blank, by BIP141. This helps ensure txid-stability of the spending transaction, which may be required for smart contracting wallets. If we do not use segwit, a miner can simply insert an `OP_NOP` in the `scriptSig` without effecting the result of program execution.
An additional relay restriction is to disallow non-empty witness data, which an adversary may use to penalize the "honest" transactor when RBF'ing the transaction due to the incremental fee requirement of RBF rules.
The intended use-case for this output type is to "anchor" the transaction with a spending child to bring exogenous CPFP fees into the transaction package, encouraging the inclusion of the package in a block. The minimal size of creation and spending of this output makes it an attractive contrast to outputs like `p2sh(OP_TRUE)` and `p2wsh(OP_TRUE)` which
are significantly larger in vbyte terms.
Combined with TRUC transactions which limits the size of child transactions significantly, this is an attractive option for presigned transactions that need to be fee-bumped after the fact.
ACKs for top commit:
sdaftuar:
utACK 75648cea5a
theStack:
re-ACK 75648cea5a
ismaelsadeeq:
re-ACK 75648cea5a via [diff](e7ce6dc070..75648cea5a)
glozow:
ACK 75648cea5a
tdb3:
ACK 75648cea5a
Tree-SHA512: d529de23d20857e6cdb40fa611d0446b49989eaafed06c28280e8fd1897f1ed8d89a4eabbec1bbf8df3d319910066c3dbbba5a70a87ff0b2967d5205db32ad1e
fa46a1b74b test: Avoid CScript() as default function argument (MarcoFalke)
fadf621825 test: Make leaf_script mandatory when scriptpath is set in TaprootSignatureMsg (MarcoFalke)
Pull request description:
Unlike other function calls in default arguments, CScript should not cause any issues in the tests, because they are const.
However, this change allows to enable the "function-call-in-default-argument (B008)" lint rule, which will help to catch severe test bugs, such as https://github.com/bitcoin/bitcoin/issues/30543#issuecomment-2259260024 .
The lint rule will be enabled in a follow-up, when all violations are fixed.
ACKs for top commit:
instagibbs:
utACK fa46a1b74b
theStack:
lgtm ACK fa46a1b74b
ismaelsadeeq:
Tested ACK fa46a1b74b
Tree-SHA512: bc68b15121d50ead0fc70ad772360a7829908aedeaff8426efcb8a67f33117f67d26b4f5da94fa735dd8de9c9ff65fc10a29323f1b12f238b75486fa7cc32a89
afd237bb5d [fuzz] Harness for version handshake (dergoegge)
a90ab4aec9 scripted-diff: Rename lazily initialized bloom filters (dergoegge)
82de1bc478 [net processing] Lazily initialize m_recent_confirmed_transactions (dergoegge)
fa0c87f19c [net processing] Lazily initialize m_recent_rejects_reconsiderable (dergoegge)
662e8db2d3 [net processing] Lazily initialize m_recent_rejects (dergoegge)
Pull request description:
This adds a fuzzing harness dedicated to the version handshake. To avoid determinism issues, the harness creates necessary components each iteration (addrman, peerman, etc). A harness like this would have easily caught https://bitcoincore.org/en/2024/07/03/disclose-timestamp-overflow/.
As a performance optimization, this PR includes a change to `PeerManager` to lazily initialize various filters (to avoid large unnecessary memory allocations each iteration).
ACKs for top commit:
brunoerg:
ACK afd237bb5d
marcofleon:
Tested ACK afd237bb5d. I compared the coverage of `net_processing` from this harness to the `process_message` and `process_messages` harnesses to see the differences. This target hits more specific parts of the version handshake. The stability looks good as well, at about 94%.
glozow:
utACK afd237bb5d lazy blooms look ok
mzumsande:
Code Review ACK afd237bb5d
Tree-SHA512: 62bba20aec0cd220e62368354891f9790b81ad75e8adf7b22a76a6d4663bd26aedc4cae8083658a75ea9043d60aad3f0e58ad36bd7bbbf93ff1d16e317bf15cc
This does not cause any issues, because CScript in the tests are const.
However, this change allows to enable the
"function-call-in-default-argument (B008)" lint rule.
a6efc7e16e test: fix intermittent failures in feature_proxy.py (Martin Zumsande)
Pull request description:
Fixes#29871
If addnode connections are made with v2transport and the peer immediately disconnects us, reconnections with v1 are scheduled. This could interfere with later checks depending on timing. Avoid this by using `v2transport=False` in the addnode rpc - this test isn't about the message layer anyway, so running it with v2 would add no value.
ACKs for top commit:
maflcko:
ACK a6efc7e16e
tdb3:
cr re ACK a6efc7e16e
Tree-SHA512: 39353a392e75e4c6257d971ceecb65fb76ec6d3b121a087869831c24b767a18f57e2ae2968da445c7fa731cb03053c90df37dd2cd6e86f786ad4121bc68ca235
This removes the default value, because there should not be a use-case
to fall back to a an empty leaf_script by default. (If there was, it
could trivially be added back)
In python, if the default value is a mutable object (here: a class)
its shared over all instances, so that one instance being changed
would affect others to be changed as well.
This was likely the source of various intermittent bugs in the
functional tests.
If addnode connections are made with v2transport and the peer immediately disconnects us, reconnections
with v1 are scheduled. This could interfere with later checks depending on timing. Avoid this by using
`v2transport=False` in the addnode rpc - this test isn't about the message layer anyway, so running it
with v2 would add no value.
17845e7f21 rpc: add utxo's blockhash and number of confirmations to scantxoutset output (Luis Schwab)
Pull request description:
This PR resolves#30478 by adding two fields to the `scantxoutset` RPC:
- blockhash: the blockhash that an UTXO was created
- confirmations: the number of confirmations an UTXO has relative to the chaintip.
The rationale for the first field is that a blockhash is a much more reliable identifier than the height:
> When using the scantxoutset RPC, the current behaviour is to show the block height of the UTXO. This is not optimal, as block height is ambiguous, especially in the case of a block reorganization happening at the same instant of the query. In this case, an UTXO that does not exist would be assumed to exist, unless the chain's tip hash is recorded before the scan, and make sure it still exists after, as per https://github.com/bitcoindevkit/bdk/issues/895#issuecomment-1475766797 comment by evanlinjin.
The second one was suggested by maflcko, and I agree it's useful for human users:
> While touching this, another thing to add could be the number of confirmations? I understand that this wouldn't help machine consumers of the interface, but human callers may find it useful?
This will yield an RPC output like so:
```diff
bitcoin-cli scantxoutset start "[\"addr(bc1q5q9344vdyjkcgv79ve3tldz4jmx4lf7knmnx6r)\"]"
{
"success": true,
"txouts": 185259116,
"height": 853622,
"bestblock": "00000000000000000002e97d9be8f0ddf31829cf873061b938c10b0f80f708b2",
"unspents": [
{
"txid": "fae435084345fe26e464994aebc6544875bca0b897bf4ce52a65901ae28ace92",
"vout": 0,
"scriptPubKey": "0014a00b1ad58d24ad8433c56662bfb45596cd5fa7d6",
"desc": "addr(bc1q5q9344vdyjkcgv79ve3tldz4jmx4lf7knmnx6r)#smk4xmt7",
"amount": 0.00091190,
"coinbase": false,
"height": 852741,
+ "blockhash": "00000000000000000002eefe7e7db44d5619c3dace4c65f3fdcd2913d4945c13",
+ "confirmations": 882
}
],
"total_amount": 0.00091190
}
```
ACKs for top commit:
sipa:
utACK 17845e7f21
Eunovo:
ACK 17845e7f21
tdb3:
ACK 17845e7f21
Tree-SHA512: 02366d0004e5d547522115ef0efe6794a35978db53dda12c675cfae38197bf43f0bf89ca99a3d79e3d2cff95186015fe1ab764abb8ab82bda440ae9302ad973b
The error messages should never happen in normal operation. However, if
they do, they are helpful to return to the user to debug the issue. For
example, to notice a truncated file.
e4b0dabb21 test: add functional test for tagged MiniWallet instances (Sebastian Falbesoner)
3162c917e9 test: fix MiniWallet internal key derivation for tagged instances (Sebastian Falbesoner)
c9f7364ab2 test: fix MiniWallet script-path spend (missing parity bit in leaf version) (Sebastian Falbesoner)
7774c314fb test: refactor: return TaprootInfo from P2TR address creation routine (Sebastian Falbesoner)
Pull request description:
This PR fixes a dormant bug in MiniWallet that exists since support for P2TR was initially added in #23371 (see commit 041abfebe4).
In the course of spending the output, the leaf version byte of the control block in the witness stack doesn't set the parity bit, i.e. we were so far just lucky that the used combinations of relevant data (internal pubkey, leaf script / version) didn't result in a tweaked pubkey with odd y-parity. If that was the case, we'd get the following validation error:
`mandatory-script-verify-flag-failed (Witness program hash mismatch) (-26)`
Since MiniWallets can now optionally be tagged (#29939), resulting in different internal pubkeys, the issue is more prevalent now. Fix it by passing the parity bit, as specified in BIP341.
Can be tested with the following patch (fails on master, succeeds on PR):
```diff
diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py
index 148cc935ed..7ebe858681 100644
--- a/test/functional/test_framework/mempool_util.py
+++ b/test/functional/test_framework/mempool_util.py
@@ -42,7 +42,7 @@ def fill_mempool(test_framework, node):
# Generate UTXOs to flood the mempool
# 1 to create a tx initially that will be evicted from the mempool later
# 75 transactions each with a fee rate higher than the previous one
- ephemeral_miniwallet = MiniWallet(node, tag_name="fill_mempool_ephemeral_wallet")
+ ephemeral_miniwallet = MiniWallet(node, tag_name="fill_mempool_ephemeral_wallet3")
test_framework.generate(ephemeral_miniwallet, 1 + num_of_batches * tx_batch_size)
# Mine enough blocks so that the UTXOs are allowed to be spent
```
In addition to that, another bug is fixed where the internal key derivation failed, as not every pseudorandom hash results in a valid x-only pubkey. Fix this by treating the hash result as private key and calculate the x-only public key out of that, to be used then as internal key.
Fixes#30528.
ACKs for top commit:
glozow:
ACK e4b0dabb21
rkrux:
reACK [e4b0dab](e4b0dabb21)
hodlinator:
ACK e4b0dabb21
Tree-SHA512: a16f33f76bcb1012857cc3129438a9f6badf28aa2b1d25696da0d385ba5866b46de0f1f93ba777ed9263fe6952f98d7d9c44ea0c0170a2bcc86cbef90bf6ac58
fac0c3d4bf doc: Add release notes for two pull requests (MarcoFalke)
fa7b57e5f5 refactor: Replace ParseHashStr with FromHex (MarcoFalke)
fa90777245 rest: Reject truncated hex txid early in getutxos parsing (MarcoFalke)
fab6ddbee6 refactor: Expose FromHex in transaction_identifier (MarcoFalke)
fad2991ba0 refactor: Implement strict uint256::FromHex() (MarcoFalke)
fa103db2bb scripted-diff: Rename SetHex to SetHexDeprecated (MarcoFalke)
fafe4b8051 test: refactor: Replace SetHex with uint256 constructor directly (MarcoFalke)
Pull request description:
In `rest_getutxos` truncated txids such as `aa` or `ff` are accepted. This is brittle at best.
Fix it by rejecting any truncated (or overlarge) input.
----
Review note: This also starts a major refactor to rework hex parsing in Bitcoin Core, meaning that a few refactor commits are included as well. They are explained individually in the commit message and the work will be continued in the future.
ACKs for top commit:
stickies-v:
re-ACK fac0c3d4bf - only doc and test updates to address review comments, thanks!
hodlinator:
ACK fac0c3d4bf
Tree-SHA512: 473feb3fcf6118443435d1dd321006135b0b54689bfbbcb1697bb5811a449bef51f475c715de6911ff3c4ea3bdb75f601861ff93347bc4414d6b9e5298105dd7
25bf86a225 [test]: ensure `estimatesmartfee` default mode is `economical` (ismaelsadeeq)
41a2545046 [fees]: change `estimatesmartfee` default mode to `economical` (ismaelsadeeq)
Pull request description:
Fixes#30009
This PR changes the `estimatesmartfee` default mode to `economical`.
This was also suggested on IRC https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-04-26#1021609
- `conservative` mode: This is the `estimatesmartfee` RPC mode which considers a longer history of blocks. It potentially returns a higher fee rate and is more likely to be sufficient for the desired target, but it is not as responsive to short-term drops in the prevailing fee market.
- `economical` mode: This is the `estimatesmartfee` RPC mode where estimates are potentially lower and more responsive to short-term drops in the prevailing fee market.
Since users are likely to use the default mode, this change will reduce overestimation for many users. The conservative mode remains available for those who wish to opt-in.
For an in-depth analysis of how significantly the `conservative` mode overestimates, see
https://delvingbitcoin.org/t/bitcoind-policy-estimator-modes-analysis/964.
ACKs for top commit:
instagibbs:
reACK 25bf86a225
glozow:
ACK 25bf86a225
willcl-ark:
ACK 25bf86a225
Tree-SHA512: 78ebda667eb9c8f87dcc2f0e6c14968bd1de30358dc77a13611b186fb8427ad97d9f537bad6e32e0a1aa477ccd8c64fee4d41e19308ef3cb184ff1664e6ba8a6
Without the fix, the test could fail intermittently. For example:
node0 2024-07-22T16:31:54.104994Z [httpworker.0] [rpc/request.cpp:232] [parse] [rpc] ThreadRPCServer method=setmocktime user=__cookie__
test 2024-07-22T16:31:54.291000Z TestFramework (INFO): Sending first 4 bytes of ellswift which match network magic
test 2024-07-22T16:31:54.292000Z TestFramework (INFO): If a response is received, assertion failure would happen in our custom data_received() function
test 2024-07-22T16:31:54.292000Z TestFramework.p2p (DEBUG): Connecting to Bitcoin Node: 127.0.0.1:12644
test 2024-07-22T16:31:54.293000Z TestFramework.p2p (DEBUG): Connected & Listening: 127.0.0.1:12644
test 2024-07-22T16:31:54.588000Z TestFramework.p2p (DEBUG): sending 4050 bytes of garbage data
test 2024-07-22T16:31:54.588000Z TestFramework (INFO): Sending remaining ellswift and garbage which are different from V1_PREFIX. Since a response is
test 2024-07-22T16:31:54.588000Z TestFramework (INFO): expected now, our custom data_received() function wouldn't result in assertion failure
node0 2024-07-22T16:31:55.523868Z (mocktime: 2024-07-22T16:31:54Z) [net] [net.cpp:3764] [CNode] [net] Added connection peer=0
node0 2024-07-22T16:31:55.625145Z (mocktime: 2024-07-22T16:31:54Z) [net] [net.cpp:1814] [CreateNodeFromAcceptedSocket] [net] connection from 127.0.0.1:45154 accepted
node0 2024-07-22T16:31:55.625769Z (mocktime: 2024-07-22T16:31:54Z) [http] [httpserver.cpp:305] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:33320
node0 2024-07-22T16:31:55.626543Z (mocktime: 2024-07-22T16:31:54Z) [httpworker.1] [rpc/request.cpp:232] [parse] [rpc] ThreadRPCServer method=getpeerinfo user=__cookie__
test 2024-07-22T16:31:55.818000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_v2_misbehaving.py", line 133, in run_test
self.test_earlykeyresponse()
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_v2_misbehaving.py", line 151, in test_earlykeyresponse
self.wait_until(lambda: node0.getpeerinfo()[-1]["bytesrecv"] > 4)
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 791, in wait_until
return wait_until_helper_internal(test_function, timeout=timeout, timeout_factor=self.options.timeout_factor)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 289, in wait_until_helper_internal
if predicate():
^^^^^^^^^^^
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_v2_misbehaving.py", line 151, in <lambda>
self.wait_until(lambda: node0.getpeerinfo()[-1]["bytesrecv"] > 4)
~~~~~~~~~~~~~~~~~~~^^^^
IndexError: list index out of range
d63ef73800 test: Add loadtxoutset test with tip on snapshot block (Fabian Jahr)
c2f86d4bcb test: Remove already resolved assumeutxo todo comments (Fabian Jahr)
Pull request description:
The first commit removes three Todos that have been addressed previously (see commit message for details).
The second message resolves another todo by adding the missing test case. This is a special case of "the tip has more work than the snapshot" where the tip is the same block as the snapshot base block.
Related to #28648.
ACKs for top commit:
jrakibi:
ACK [d63ef73](d63ef73800)
achow101:
ACK d63ef73800
maflcko:
ACK d63ef73800
alfonsoromanz:
Re ACK d63ef73800
Tree-SHA512: 8d5a25fc0b26531db3a9740132694138f2103b7b42eeb1d4a64095bfc901c1372e23601c0855c7def84c8a4e185d10611e4e830c4e479f1b663ae6ed53abb130
Sebastian Falbesoner