Bitcoin Core Github
44 subscribers
119K links
Download Telegram
📝 instagibbs opened a pull request: "test: cover invalid codesep positions for signature in taproot"
(https://github.com/bitcoin/bitcoin/pull/32301)
I didn't find another spot where incorrect codesep coverage existed, so I added some and modified the serializer to allow for maximal value `0xfffffffe` since it was being serialized as a signed integer.
💬 achow101 commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r2049609432)
Done
💬 achow101 commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r2049609523)
Done
💬 achow101 commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r2049609602)
Done
💬 achow101 commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r2049609663)
Done
🤔 w0xlt reviewed a pull request: "rpc: Support v3 raw transactions creation"
(https://github.com/bitcoin/bitcoin/pull/31936#pullrequestreview-2776864390)
Changes made to `rpc_rawtransaction.py` in https://github.com/bitcoin/bitcoin/pull/31936/commits/d0453049f5d6678096f3575f6d595467506a05f1 in the functions `raw_multisig_transaction_legacy_tests` and `raw_multisig_transaction_legacy_tests` does not have any effect.

A better approach might be to validate a v3 transaction in `createrawtransaction_tests`, for example, as shown below.

```diff
# Multiple mixed outputs
tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[
...
🤔 w0xlt reviewed a pull request: "rpc: Allow fullrbf fee bump in (psbt)bumpfee"
(https://github.com/bitcoin/bitcoin/pull/31953#pullrequestreview-2776921300)
ACK https://github.com/bitcoin/bitcoin/pull/31953/commits/fa86190e6ed2aeda7bcceaf96f52403816bcd751

CI error seems to be unrelated
💬 sipa commented on pull request "feefrac: avoid integer overflow in temporary":
(https://github.com/bitcoin/bitcoin/pull/32300#discussion_r2049668593)
I have added this in the unit tests:

```c++
// Test for integer overflow issue (https://github.com/bitcoin/bitcoin/issues/32294)
BOOST_CHECK_EQUAL(FeeFrac{0x7ffffffdfffffffb, 0x7ffffffd}.EvaluateFeeDown(0x7fffffff), 0x7fffffffffffffff);
```

But note that to trigger it, you need to build with sanitizers (which are generally only enabled in fuzz binaries, I think)?
🤔 fjahr reviewed a pull request: "psbt: MuSig2 Fields"
(https://github.com/bitcoin/bitcoin/pull/31247#pullrequestreview-2776864149)
Code review ACK f385ff7aba0eab6cf00ede1b808817dfd7047399

Typo may be fixed in the next MuSig2 PR unless you want to retouch.
💬 fjahr commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r2049610253)
nit: s/particitpant/participant/
💬 galaxycores commented on pull request "bench: Fix WalletMigration benchmark":
(https://github.com/bitcoin/bitcoin/pull/32281#discussion_r2049674204)
AdToWallet(tx)
💬 galaxycores commented on pull request "bench: Fix WalletMigration benchmark":
(https://github.com/bitcoin/bitcoin/pull/32281#discussion_r2049676386)
(WriteWachOnly;)
💬 l0rinc commented on pull request "feefrac: avoid integer overflow in temporary":
(https://github.com/bitcoin/bitcoin/pull/32300#discussion_r2049680441)
```C++
BOOST_CHECK_EQUAL((FeeFrac{0x7ffffffdfffffffb, 0x7ffffffd}.EvaluateFeeDown(0x7fffffff)), 0x7fffffffffffffff);
```
This works for me in simple Debug mode (with extra parentheses, built without sanitizers, doesn't fail in Release mode).
Before the fix this also fails with:
> cmake -B build -DCMAKE_BUILD_TYPE=Debug && cmake --build build -j$(nproc) && time build/bin/test_bitcoin --run_test='feefrac_tests/feefrac_operators'
...
> Running 1 test case...
> zsh: trace trap
💬 l0rinc commented on pull request "feefrac: avoid integer overflow in temporary":
(https://github.com/bitcoin/bitcoin/pull/32300#issuecomment-2814074180)
Tested ACK 5cb1241814b9c541c5e5e8daadbbea595f2960ae
💬 sipa commented on pull request "test: cover invalid codesep positions for signature in taproot":
(https://github.com/bitcoin/bitcoin/pull/32301#issuecomment-2814090563)
ACK 175740991e0081c35f471cd7f1ad067e3e1f978f
💬 mzumsande commented on pull request "test: allow all functional tests to be run or skipped with --usecli":
(https://github.com/bitcoin/bitcoin/pull/32290#issuecomment-2814098958)
> If this ends up fixing all issues, should add --usecli to atleast one CI, so we catch regressions?

Yes - it'd be a bit of a waste of a CI job if we end up disabling too many of the tests with `--usecli`, but depending on how many can be fixed (I'm currently still going through all the existing disabled tests) that would make sense to me.
💬 l0rinc commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#issuecomment-2814103775)
The fuzzer still has some problems with `CCoinsViewCache`:
> runtime error: unsigned integer overflow: 0 - 48 cannot be represented in type 'size_t' (aka 'unsigned long')
[15:43:31.228] #0 0x558e624b3787 in CCoinsViewCache::SpendCoin(COutPoint const&, Coin*) /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/./coins.cpp:133:22
[15:43:31.228]

I'm not yet sure how that's possible, `FetchCoin` should populate the `cachedCoinsUsage` field if it's missing - will try to reproduce it
...
📝 l0rinc converted_to_draft a pull request: "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`"
(https://github.com/bitcoin/bitcoin/pull/32296)
Inspired by https://github.com/bitcoin/bitcoin/pull/32154, the main goal of this PR is to reenable sanitizer checks for `serialize.h` (last commit)
💬 achow101 commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r2049755996)
Oops, fixed
💬 ariard commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/31989#discussion_r2049770396)
Thanks for the `bench.cpp`, I had a look on `benchmark_sha256` and `benchmark_signature_validation`.

Though here few more thoughts on the 1-bit cost from the view of a DoSing peer.

Currently, we have 2 caches the signature cache and the `m_script_execution_cache`.

If a script has already an entry at `CheckInputScripts()`, bitcoin core returns true:
https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L2185

If there is already a cache entry, and the script includes signa
...