fa5cc3ccfb test: Fix intermittent issue in mempool_reorg (MarcoFalke)
Pull request description:
Currently the test case may fail intermittently, see https://github.com/bitcoin/bitcoin/issues/28313
Fix this by changing a number and reducing the failure rate a bit.
ACKs for top commit:
glozow:
ACK fa5cc3ccfb
Tree-SHA512: ff552111b434ca712c7dbdc5ba32a986a6fa4512cba5a756234eae69428063bf6ecfdc8f350ee84226ed4d3e4262b4639dbe49162a722e8da85f0d61e5690c51
fa15f7e082 ci: Remove no longer applicable section (MarcoFalke)
fa378bed56 ci: Start with clean env (MarcoFalke)
fa8c250c2f ci: Limit scope of some env vars (MarcoFalke)
Pull request description:
Starting with a clean `env` should help to avoid non-determinism, such as the one fixed in https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564529747
ACKs for top commit:
dergoegge:
utACK fa15f7e082
Tree-SHA512: 716b264217557b6524dab92d5a2a8d61ecb982dff475bd0cf5a763070b4c5916cd5995e764eb5d67d9cf2428c29d5fc2f42b32941b54c7c3053123ce448171e5
Using the new time-machine results in warnings about consistently using
keyword arguments:
```bash
guix environment: warning: 'cross-kernel-headers' must be used with keyword arguments
guix environment: warning: 'cross-libc' must be used with keyword arguments
```
This is required for bumping the time-machine, for compatibility with
OpenSSL:
oscrypto: openssl backend, 1.2.1, /tmp/guix-build-python-oscrypto-1.2.1.drv-0/source/oscrypto
Traceback (most recent call last):
File "/tmp/guix-build-python-oscrypto-1.2.1.drv-0/source/oscrypto/_openssl/_libcrypto_ctypes.py", line 304, in <module>
libcrypto.EVP_PKEY_size.argtypes = [
File "/gnu/store/9dkl9fnidcdpw19ncw5pk0p7dljx7ijb-python-3.10.7/lib/python3.10/ctypes/__init__.py", line 387, in __getattr__
func = self.__getitem__(name)
File "/gnu/store/9dkl9fnidcdpw19ncw5pk0p7dljx7ijb-python-3.10.7/lib/python3.10/ctypes/__init__.py", line 392, in __getitem__
func = self._FuncPtr((name_or_ordinal, self))
AttributeError: /gnu/store/2hr7w64zhr6jjznidyc2xi40d5ynhj9c-openssl-3.0.8/lib/libcrypto.so.3: undefined symbol: EVP_PKEY_size. Did you mean: 'EVP_PKEY_free'?
806b75b213 guix: consolidate Linux GCC package (fanquake)
4415275f96 guix: consolidate glibc 2.27 package (fanquake)
Pull request description:
This is some refactoring to the Linux Guix build that facilitates bumping our Guix time-machine. Namely, avoiding `package-with-extra-configure-variable`, which is non-functional in the newer time-machine, see https://issues.guix.gnu.org/64436.
At the same time, consolidate our Linux GCC build into `linux-base-gcc`. Now that we only use `building-on`, remove `explicit-cross-configure`.
Split out of https://github.com/bitcoin/bitcoin/pull/27897. Most of the [[WIP] Linux commit](8335fc4775), minus anything GCC 12 related.
I'll also be splitting out the other changes we can do pre-timemachine bump, for easier review.
Similar/followup to #28294. Requirement for #28328.
Guix Build:
```bash
17463110d4b4721a7c188e71b1fc00c9b5b82227aa8342471390c17678e04a9a guix-build-806b75b21340/output/aarch64-linux-gnu/SHA256SUMS.part
0ca919ce568e7d4ffe44dda871d48963ca2988516068e75b1f30ca342d853d20 guix-build-806b75b21340/output/aarch64-linux-gnu/bitcoin-806b75b21340-aarch64-linux-gnu-debug.tar.gz
134afa263e4da6c8d7df79a7dd8e59911c1e643b53b7c285de9418d97fb06d5f guix-build-806b75b21340/output/aarch64-linux-gnu/bitcoin-806b75b21340-aarch64-linux-gnu.tar.gz
22ce318241084829e57f064bf47de57752151863aac545e643eea5dd8eee96fe guix-build-806b75b21340/output/arm-linux-gnueabihf/SHA256SUMS.part
a26fecfeb821040704ca70ea056bff796789ed9500d9575d8fa13a85b32143f6 guix-build-806b75b21340/output/arm-linux-gnueabihf/bitcoin-806b75b21340-arm-linux-gnueabihf-debug.tar.gz
213c84494835c81a40ebc5d38a62bb19cbee5b214b2a8aaed6d28746b245108e guix-build-806b75b21340/output/arm-linux-gnueabihf/bitcoin-806b75b21340-arm-linux-gnueabihf.tar.gz
ce1efcf6d3ca0e7422c5ce35f5e45e0770a3ae48173e061137daa7dc551e9d48 guix-build-806b75b21340/output/arm64-apple-darwin/SHA256SUMS.part
fc01aaeb4e4722d21fd60c78f1b5322c9875ec6fb4d244f4547a354e91a33ed7 guix-build-806b75b21340/output/arm64-apple-darwin/bitcoin-806b75b21340-arm64-apple-darwin-unsigned.dmg
632e4a243d3e4144313f53047499f91b7c9380a1a50f5846e1635d0a00fd202a guix-build-806b75b21340/output/arm64-apple-darwin/bitcoin-806b75b21340-arm64-apple-darwin-unsigned.tar.gz
8e694e4cd1bf45e6a586a0d8c19b675014f168f342f881a9ae0c4fbbda796914 guix-build-806b75b21340/output/arm64-apple-darwin/bitcoin-806b75b21340-arm64-apple-darwin.tar.gz
dad6e8475f13dac6c4f0b182f53dc330997e9e1e5cf4d46413655f319dcd9bff guix-build-806b75b21340/output/dist-archive/bitcoin-806b75b21340.tar.gz
32e8b6c7e7a7561e132c5f15e2151a51aad1c5004ab90a36a7e80f92c200ef6f guix-build-806b75b21340/output/powerpc64-linux-gnu/SHA256SUMS.part
9033e85e03bd12a3a19599735cfd44fcfdfb1bf1b632733341cec6a4f75ff86f guix-build-806b75b21340/output/powerpc64-linux-gnu/bitcoin-806b75b21340-powerpc64-linux-gnu-debug.tar.gz
72698691b27ec0ac17f21dce8551de0ca683dd00b5b9442ea7616fb56cca8c6b guix-build-806b75b21340/output/powerpc64-linux-gnu/bitcoin-806b75b21340-powerpc64-linux-gnu.tar.gz
ef7c6d7184249eb59fa67d6df91d1a567570b9fb026dbb8682763029decaacca guix-build-806b75b21340/output/powerpc64le-linux-gnu/SHA256SUMS.part
fc6bb5619ee76899a88c1dd62640b429ad8957bbdf821238038b41fc87d18eca guix-build-806b75b21340/output/powerpc64le-linux-gnu/bitcoin-806b75b21340-powerpc64le-linux-gnu-debug.tar.gz
0eceb969f41f6b8dba88f641e268590de7edf0008318c8051d9cb208fb15e7f7 guix-build-806b75b21340/output/powerpc64le-linux-gnu/bitcoin-806b75b21340-powerpc64le-linux-gnu.tar.gz
6f51a4791d87a610abd81cee83efa7f469e905829797bc2edac4fb95a2e0f3e4 guix-build-806b75b21340/output/riscv64-linux-gnu/SHA256SUMS.part
c978706988f31e65a7991ff7582d79b3d1df44249c14d9807d93c01bf3f5080d guix-build-806b75b21340/output/riscv64-linux-gnu/bitcoin-806b75b21340-riscv64-linux-gnu-debug.tar.gz
93aefe058025067550692adae59ead14228ac252a9e7cf8b55c8fb4189ece545 guix-build-806b75b21340/output/riscv64-linux-gnu/bitcoin-806b75b21340-riscv64-linux-gnu.tar.gz
862a53f6023bd1ca98a078ea540bba8ca9bfa335a9560f3d8d62ac873c2d5848 guix-build-806b75b21340/output/x86_64-apple-darwin/SHA256SUMS.part
8f632b42c94d061fa30364087e75bb8b04eb2ac5a0a988eacc37c5983669f01b guix-build-806b75b21340/output/x86_64-apple-darwin/bitcoin-806b75b21340-x86_64-apple-darwin-unsigned.dmg
ce62e76ca446a6316b31490e12463c0a641e15beef9bdae7acc8e5db057b433a guix-build-806b75b21340/output/x86_64-apple-darwin/bitcoin-806b75b21340-x86_64-apple-darwin-unsigned.tar.gz
f57b014818e3b1ec07d27c8224ec4ac0e5786dacd00639513b599c6138790ece guix-build-806b75b21340/output/x86_64-apple-darwin/bitcoin-806b75b21340-x86_64-apple-darwin.tar.gz
70e75f6f13795f968f91719d221673b687bf747f90d77912cbcb2c1ee45ec623 guix-build-806b75b21340/output/x86_64-linux-gnu/SHA256SUMS.part
30dec525364bb21a26cfe8bfff061d013c4ce849165aa67b06eb154019444862 guix-build-806b75b21340/output/x86_64-linux-gnu/bitcoin-806b75b21340-x86_64-linux-gnu-debug.tar.gz
d8b3a996f25fb948b3555d5750852aaf82f7051848586b9ba0f4d0d223226e4b guix-build-806b75b21340/output/x86_64-linux-gnu/bitcoin-806b75b21340-x86_64-linux-gnu.tar.gz
4259adec77912bab6494f71a2a95d98093b116c05fc9ad03069e92de4ce0248c guix-build-806b75b21340/output/x86_64-w64-mingw32/SHA256SUMS.part
0a2d5cab3fe94a86def0cc1b6efe9ac871839cbcdc05ad92686df1d2bdd154ea guix-build-806b75b21340/output/x86_64-w64-mingw32/bitcoin-806b75b21340-win64-debug.zip
d2a1876333bdb1cd5b8b1d4a52bccd756ea2e992c291dac233e65beeb0c905fd guix-build-806b75b21340/output/x86_64-w64-mingw32/bitcoin-806b75b21340-win64-setup-unsigned.exe
192ea38d70e12c23327ff811ea930b50ac31c9fb2bc8dcc9391ad585112322ff guix-build-806b75b21340/output/x86_64-w64-mingw32/bitcoin-806b75b21340-win64-unsigned.tar.gz
474f88a1f4cc8900a7d8967909336d4122e449ce98cacaf2cacec340780ede0b guix-build-806b75b21340/output/x86_64-w64-mingw32/bitcoin-806b75b21340-win64.zip
```
ACKs for top commit:
TheCharlatan:
Nice cleanups, ACK 806b75b213
Tree-SHA512: cede797c3b9b88cc1588d0ff7ff9b2908316a8ba384d9087b16466aceeb2e0c194aa56e3023f6b6ce7ca8896a1b87ef56b966db198cc1712cb6ddc37fe684567
For the sake of building previous releases, fetching the whole history
of the repository for each version seems to be overkill as it takes much
more time, bandwidth and disk space than necessary. Create a shallow
clone instead with history truncated to the one commit of the version
tag, which is directly checked out in the same command. This has the
nice side-effect that we can remove the extra `git checkout` step after
as it's not needed anymore.
Note that it might look confusing to pass a _tag_ to a parameter named
`--branch`, but the git-clone manpage explicitly states that this is
supported.
This furthers transport abstraction by removing the assumption that a message
can always immediately be converted to wire bytes. This assumption does not hold
for the v2 transport proposed by BIP324, as no messages can be sent before the
handshake completes.
This is done by only keeping (complete) CSerializedNetMsg objects in vSendMsg,
rather than the resulting bytes (for header and payload) that need to be sent.
In SocketSendData, these objects are handed to the transport as permitted by it,
and sending out the bytes the transport tells us to send. This also removes the
nSendOffset member variable in CNode, as keeping track of how much has been sent
is now a responsability of the transport.
This is not a pure refactor, and has the following effects even for the current
v1 transport:
* Checksum calculation now happens in SocketSendData rather than PushMessage.
For non-optimistic-send messages, that means this computation now happens in
the network thread rather than the message handler thread (generally a good
thing, as the message handler thread is more of a computational bottleneck).
* Checksum calculation now happens while holding the cs_vSend lock. This is
technically unnecessary for the v1 transport, as messages are encoded
independent from one another, but is untenable for the v2 transport anyway.
* Statistics updates about per-message sent bytes now happen when those bytes
are actually handed to the OS, rather than at PushMessage time.
This more accurately captures the intent of limiting send buffer size, as
many small messages can have a larger overhead that is not counted with the
current approach.
It also means removing the dependency on the header size (which will become
a function of the transport choice) from the send buffer calculations.
This adds a simulation test, with two V1Transport objects, which send messages
to each other, with sending and receiving fragmented into multiple pieces that
may be interleaved. It primarily verifies that the sending and receiving side
are compatible with each other, plus a few sanity checks.
The rest of net.cpp already uses Params() to determine chainparams in many
places (and even V1Transport itself does so in some places).
Since the only chainparams dependency is through the message start characters,
just store those directly in the transport.
This makes the sending side of P2P transports mirror the receiver side: caller provides
message (consisting of type and payload) to be sent, and then asks what bytes must be
sent. Once the message has been fully sent, a new message can be provided.
This removes the assumption that P2P serialization of messages follows a strict structure
of header (a function of type and payload), followed by (unmodified) payload, and instead
lets transports decide the structure themselves.
It also removes the assumption that a message must always be sent at once, or that no
bytes are even sent on the wire when there is no message. This opens the door for
supporting traffic shaping mechanisms in the future.
Now that the Transport class deals with both the sending and receiving side
of things, make the receive side have function names that clearly indicate
they're about receiving.
* Transport::Read() -> Transport::ReceivedBytes()
* Transport::Complete() -> Transport::ReceivedMessageComplete()
* Transport::GetMessage() -> Transport::GetReceivedMessage()
* Transport::SetVersion() -> Transport::SetReceiveVersion()
Further, also update the comments on these functions to (among others) remove
the "deserialization" terminology. That term is better reserved for just the
serialization/deserialization between objects and bytes (see serialize.h), and
not the conversion from/to wire bytes as performed by the Transport.
Rather than relying on the caller to prevent concurrent calls to the
various receive-side functions of Transport, introduce a private m_cs_recv
inside the implementation to protect the lock state.
Of course, this does not remove the need for callers to synchronize calls
entirely, as it is a stateful object, and e.g. the order in which Receive(),
Complete(), and GetMessage() are called matters. It seems impossible to use
a Transport object in a meaningful way in a multi-threaded way without some
form of external synchronization, but it still feels safer to make the
transport object itself responsible for protecting its internal state.
This allows state that is shared between both directions to be encapsulated
into a single object. Specifically the v2 transport protocol introduced by
BIP324 has sending state (the encryption keys) that depends on received
messages (the DH key exchange). Having a single object for both means it can
hide logic from callers related to that key exchange and other interactions.
27b168b81f Update help text for spend and rawtransaction rpcs (Michael Tidwell)
Pull request description:
The "data" field without outputs was marked as "required" in the help docs when using bitcoin-cli. This field when left off worked as an intended optional OP_RETURN. closes #27828.
Motivation: It is hard to understand that "data" is actually optional for commands like `createpsbt` and `walletcreatefundedpsbt`.
ACKs for top commit:
achow101:
ACK 27b168b81f
Sjors:
tACK 27b168b81f
Tree-SHA512: 235e7ed4af69880880c04015b3f7de72c8f31ae035485c4c64c483e282948f3ea3f1eef16f15e260a1aaf21114150713516ba6a99967ccad9ecd91ff67cb0450
1b09cc5959 Make post-p2sh consensus rules mandatory for tx relay (Anthony Towns)
69c31bc748 doc, policy: Clarify comment on STANDARD_SCRIPT_VERIFY_FLAGS (Anthony Towns)
Pull request description:
The `MANDATORY_SCRIPT_VERIFY_FLAGS` constant was introduced in #3843 to distinguish between block consensus rules and relay standardness rules. However it was not actually used in the consensus code path: instead it only differentiates between the failure being reported as `TX_CONSENSUS` and `mandatory-script-verify-flag-failed` vs `TX_NOT_STANDARD` and `non-mandatory-script-verify-flag`.
This updates the list of mandatory flags to include the post-p2sh soft forks that are enforced as consensus rules via `GetBlockScriptFlags()`. The effect of this change is that validation.cpp will report `TX_CONSENSUS` failures for txs that fail dersig/csv/cltv/nulldummy/witness/taproot checks, instead of `TX_NOT_STANDARD`, which in turn adds `Misbehaving(100)` via `MaybePunishNodeForTx` in `net_processing`.
ACKs for top commit:
Sjors:
Code review ACK 1b09cc5959
darosior:
ACK 1b09cc5959
achow101:
ACK 1b09cc5959
theStack:
Concept and code-review ACK 1b09cc5959
Tree-SHA512: d3e5868e8cece478f2e934956ba0c231d8bb9c2daefd0df1f817774e292049902cfc1d0cd76dbd2e7722627a93eab2d7046ff678199aac70a2b01642e69349f1
The valid results should have a target below the sum of
the selected inputs amounts. Also, it increases the
minimum value for target to make it more realistic.
Instead of using `cost_of_change` for `min_viable_change`
and `change_cost`, and 0 for `change_fee`, use values from
`coin_params`. The previous values don't generate any effects
that is relevant for that context.
fa8e89d5e4 ci: Remove distro-name from task name (MarcoFalke)
fad006fa0a ci: Switch remaining tasks to self-hosted (MarcoFalke)
Pull request description:
Cirrus CI will be capping the free compute soon. For now, switch more tasks to persistent worker, as recommended by Cirrus CI.
(See slightly related discussion in https://github.com/bitcoin/bitcoin/issues/28098)
ACKs for top commit:
pinheadmz:
concept ACK fa8e89d5e4
dergoegge:
ACK fa8e89d5e4
hebasto:
ACK fa8e89d5e4.
Tree-SHA512: f6b172fee14856021b7a219b2490c8a163ad0137567c34a383080c68f8319b1d846923e508a968f43fb63ed6ce536dcb0611905fa288271f2267764b07bf9ecb
The failure arises because the test expects 'init_wallet()' (the test
framework function) creating a wallet with no keys. However, the function
also imports the deterministic private key used to receive the coinbase coins.
This causes a race within the "restore using dumped wallet" case, where we
intend to have a new wallet (with no existing keys) to test the
'importwallet()' RPC result.
The reason behind the intermittent failures might be other peers delivering
the chain right after node2 startup (sync of the validation queue included)
and prior to the 'node2.getbalance()' check.
9eac5a0529 [functional test] transaction orphan handling (glozow)
61e77bb901 [test framework] make it easier to fast-forward setmocktime (glozow)
Pull request description:
I was doing some mutation testing (through reckless refactoring) locally and found some specific behaviors in orphan handling that weren't picked up by tests. Adding some of these test cases now can maybe help with reviewing refactors like #28031.
- Parent requests aren't sent immediately. A delay is added and the requests are filtered by AlreadyHaveTx before they are sent, which means you can't use fake orphans to probe precise arrival timing of a tx.
- Parent requests include all that are not AlreadyHaveTx. This means old confirmed parents may be requested.
- The node does not give up on orphans if the peer responds to a parent request with notfound. This means that if a parent is an old confirmed transaction (in which notfound is expected), the orphan should still be resolved.
- Rejected parents can cause an orphan to be dropped, but it depends on the reason and only based on txid.
- Rejected parents can cause an orphan to be rejected too, by both wtxid and txid.
- Requests for orphan parents should be de-duplicated with "regular" txrequest. If a missing parent has the same hash as an in-flight request, it shouldn't be requested.
- Multiple orphans with overlapping parents should not cause duplicated parent requests.
ACKs for top commit:
instagibbs:
reACK 9eac5a0529
dergoegge:
reACK 9eac5a0529
achow101:
ACK 9eac5a0529
fjahr:
Code review ACK 9eac5a0529
Tree-SHA512: 85488dc6a3f62cf0c38e7dfe7839c01215b44b172d1755b18164d41d01038f3a749451241e4eba8b857fd344a445740b21d6382c45977234b21460e3f53b1b2a
