And under the hood suppoert single transactions
in AcceptPackage. This simplifies user experience
and paves the way for reducing number of codepaths
for transaction acceptance in the future.
Co-Authored-By: instagibbs <gsanders87@gmail.com>
* Since the main LIMITED_WHILE stated `outpoints.size() < 200'000`, I've presized outpoints accordingly.
* `tx_mut.vin` and `tx_mut.vout` weren't caught by the clang-tidy, but addressed them anyway.
This was missed during the original PR switching the nHashType argument
to int32_t in SignatureHash in bc52cda1f3.
The problem was discovered after running into a linker error when
attempting to link this code as a static library using the header as a
declaration with a riscv32 bare metal toolchain. The compiler would
error with:
/opt/riscv-ilp32/lib/gcc/riscv32-unknown-elf/13.2.0/../../../../riscv32-unknown-elf/bin/ld: build_kernel_riscv/src/libbitcoin_consensus.a(interpreter.cpp.o): in function `GenericTransactionSignatureChecker<CTransaction>::CheckECDSASignature(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, CScript const&, SigVersion) const':
/home/user/bitcoin/build_kernel_riscv/./script/interpreter.cpp:2043:(.text._ZNK34GenericTransactionSignatureCheckerI12CTransactionE19CheckECDSASignatureERKSt6vectorIhSaIhEES6_RK7CScript10SigVersion[_ZNK34GenericTransactionSignatureCheckerI12CTransactionE19CheckECDSASignatureERKSt6vectorIhSaIhEES6_RK7CScript10SigVersion]+0xee): undefined reference to `uint256 SignatureHash<CTransaction>(CScript const&, CTransaction const&, unsigned int, int, long long const&, SigVersion, PrecomputedTransactionData const*)'
Can be tested by running
```
$ sudo tcpdump -i eth0 host 11.22.33.44
```
and verifying that no packets appear in the tcpdump output.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
We build the only moreutils utility we actually need (sponge), have less
unused stuff in the Guix environment, and, the dependency graph is
simplified. i.e we no-longer have a dependency on perl, docbook etc, for
this package.
The `-ffile-prefix-map` compiler option implies `-fprofile-prefix-map`
on GCC or `-fcoverage-prefix-map` on Clang, which can lead to issues
with coverage builds.
This change applies only the options necessary for build reproducibility
and accurate source location messages.
This tests the new submitblock behaviour that is introduced in the
previous commit: Submitting a previously pruned block should persist the
block's data again.
The duplicate checks are repeated early in the contextual checks of
ProcessNewBlock. If duplicate blocks are detected much of their
validation is skipped. Depending on the constitution of the block,
validating the merkle root of the block is part of the more intensive
workload when validating a block. This could be an argument for moving
the pre-checks into block processing. In net_processing this would have
a smaller effect however, since the block mutation check, which also
validates the merkle root, is done before.
A side effect of this change is that a duplicate block is persisted
again on disk even when pruning is activated. This is similar to the
behaviour with getblockfrompeer. Add a release note for this change in
behaviour.
Testing spamming a node with valid, but duplicate unrequested blocks
seems to exhaust a CPU thread, but does not seem to significantly impact
keeping up with the tip. The benefits of adding these checks to
net_processing are questionable, especially since there are other ways
to trigger the more CPU-intensive checks without submitting a duplicate
block. Since these DOS concerns apply even less to the RPC interface,
which does not have banning mechanics built in, remove them too.
---
With the introduction of a mining ipc interface and the potential future
introduction of a kernel library API it becomes increasingly important
to offer common behaviour between them. An example of this is
ProcessNewBlock, which is used by ipc, rpc, net_processing and
(potentially) the kernel library. Having divergent behaviour on
suggested pre-checks and checks for these functions is confusing to both
developers and users and is a maintenance burden.
The rpc interface for ProcessNewBlock (submitblock) currently pre-checks
if the block has a coinbase transaction and whether it has been
processed before. While the current example binary for how to use the
kernel library, bitcoin-chainstate, imitates these checks, the other
interfaces do not.
ProcessNewBlock fails if an invalid duplicate block is passed in through
its call to AcceptBlock and AcceptBlockHeader. The failure in
AcceptBlockHeader makes AcceptBlock return early. This makes the
pre-check in submitblock redundant.
---
With the introduction of a mining ipc interface and the potential future
introduction of a kernel library API it becomes increasingly important
to offer common behaviour between them. An example of this is
ProcessNewBlock, which is used by ipc, rpc, net_processing and
(potentially) the kernel library. Having divergent behaviour on
suggested pre-checks and checks for these functions is confusing to both
developers and users and is a maintenance burden.
The rpc interface for ProcessNewBlock (submitblock) currently pre-checks
if the block has a coinbase transaction and whether it has been
processed before. While the current example binary for how to use the
kernel library, bitcoin-chainstate, imitates these checks, the other
interfaces do not.
The coinbase check is repeated again early during ProcessNewBlock.
Pre-checking it may also shadow more fundamental problems with a block.
In most cases the block header is checked first, before validating the
transactions. Checking the coinbase first therefore masks potential
issues with the header. Fix this by removing the pre-check.
The pre-check was likely introduced on top of
ada0caa165 to fix UB in
GetWitnessCommitmentIndex in case a block's transactions are empty. This
code path could only be reached because of the call to
UpdateUncommittedBlockStructures in submitblock, but cannot be reached
through net_processing.
Add some functional test cases to cover the previous conditions that
lead to a "Block does not start with a coinbase" json rpc error being
returned.
---
With the introduction of a mining ipc interface and the potential future
introduction of a kernel library API it becomes increasingly important
to offer common behaviour between them. An example of this is
ProcessNewBlock, which is used by ipc, rpc, net_processing and
(potentially) the kernel library. Having divergent behaviour on
suggested pre-checks and checks for these functions is confusing to both
developers and users and is a maintenance burden.
The rpc interface for ProcessNewBlock (submitblock) currently pre-checks
if the block has a coinbase transaction and whether it has been
processed before. While the current example binary for how to use the
kernel library, bitcoin-chainstate, imitates these checks, the other
interfaces do not.
92d3d691f0 fuzz: Implement G_TEST_GET_FULL_NAME (Hodlinator)
Pull request description:
Catching up to bench & unit tests. Makes for more orderly paths for fuzz tests using `BasicTestingSetup`.
### Before
```
/tmp/test_common bitcoin/0748ae43ef8fa80703bc/regtest/blocks/xor.dat
```
### After
```
/tmp/test_common bitcoin/tx_pool_standard/f18b3744625e0600eb0c/regtest/blocks/xor.dat
```
ACKs for top commit:
kevkevinpal:
ACK [92d3d69](92d3d691f0)
furszy:
utACK 92d3d691f0
tdb3:
ACK 92d3d691f0
dergoegge:
utACK 92d3d691f0
brunoerg:
code review ACK 92d3d691f0
Tree-SHA512: 5e83970b111232adece10f79e3a43d0c3c49ab635763e2a4b420f1336cbb8fee94aab751264ddec01ac8363166636e3b29cfe3b2969fc28c8dd6b31bda351950
fe3457ccff ci: note that we should install pkgconf in future (fanquake)
8d203480b3 doc: migrate from pkg-config to pkgconf in macOS build docs (fanquake)
Pull request description:
Migrate the macOS build docs and CI from `pkg-config` to `pkgconf`. As the former now just redirects to the later.
Upstream is currently mass-migrating its formula. i.e https://github.com/Homebrew/homebrew-core/pull/198317.
Fixes#31334.
ACKs for top commit:
maflcko:
ACK fe3457ccff🍭
hebasto:
re-ACK fe3457ccff.
Tree-SHA512: 6e337acb6767d163491149b6ae7181d7d7042bc11cdc745eb6f52d4df6d7a19c4f6daa000b314acd9178f97e670aba145f829e48b1b3033117d7e39cdd3af177
Recently added mempool_util implementation probably evolved in parallel with the package RBF one before being submitted as part of ephemeral dust in e2e30e89ba.
Brew has migrated to using the later:
```bash
brew info pkg-config
==> pkgconf: stable 2.3.0 (bottled), HEAD
Package compiler and linker metadata toolkit
https://github.com/pkgconf/pkgconf
```
