Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 sipa commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1949816655)
@instagibbs Note that this is about a separate addition I made, which compares the behaviour with a pure floating-point simulation. The comparison with `CFeeRate` is elsewhere.
💬 instagibbs commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1949817770)
oh right, I conflated the two threads here
💬 furszy commented on issue "Prune Node Rescan Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/29183#issuecomment-2649137150)
> Since we do need to selectively re-download specific blocks, it might be prudent to have an option only re-download those specific blocks from peers only over TOR

Thats possible for sure. Could also request a few extra blocks to make the guess harder.

> Also, can the re-downloaded blocks be re-downloaded in parallel since we will know what blocks they are?

Yes. Thats how #27837 was implemented.
💬 brunoerg commented on issue "wallet: Branch and Bound producing change":
(https://github.com/bitcoin/bitcoin/issues/31830#issuecomment-2649180340)
friendly ping: @murchandamus
👍 stickies-v approved a pull request: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/31422#pullrequestreview-2607160362)
ACK 25f150304d01940d7936125b7b4bf8e6c01d35aa

I've verified that all backported commits are clean, and they seem sensible. The CI failure seems unrelated, and I could not reproduce locally with `./test/functional/test_runner.py --previous-releases --filter wallet_upgradewallet`. It was reported in https://github.com/bitcoin/bitcoin/issues/31210 and fixed in https://github.com/bitcoin/bitcoin/pull/30125, but backporting the (quite extensive) fix commit probably is not necessary for this PR?
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2649234327)
Pushed a fix for the wrong architecture detection.
💬 yancyribbens commented on issue "wallet: Branch and Bound producing change":
(https://github.com/bitcoin/bitcoin/issues/31830#issuecomment-2649274408)
Change is computed as:

```
effective_values - target - change_fee
1395186823938825 - 1395186823938466 - 155
204
```

Why does the test assert this should be zero instead of 204?
🤔 instagibbs reviewed a pull request: "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs"
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-2606517127)
The resource bounds additions seem to make sense, still working through the workset change implications.

I've got a minimal fuzz harness checking that the "honest" peer cannot be evicted, please feel free to take it: https://github.com/instagibbs/bitcoin/tree/2025-01-orphanage-peer-dos_greg_2
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1949514690)
s/may provide/may provide without risking eviction/?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1949514831)
s/allocation/protected allocation/?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1949498021)
ef2f44e653a4877d4e65fbd5a51ec83ceb96d212

`MAX_GLOBAL_ORPHAN_ANNOUNCEMENTS` doesn't occur in the PR, should be `DEFAULT_MAX_ORPHAN_ANNOUNCEMENTS`?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1949538646)
note: this is use to "normalize" the one DoS score metric against the other only. If this wasn't used, trimming would heavily favor announcement trimming scores first.
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1949519057)
do we think we'll end up using the derived class in a meaningful way later?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1949554535)
re:global memory limits, they won't be increased until each connected peer offers up their own orphan or announcement (which makes sense)

would be good to have test/fuzz coverage that there isn't some bug where this continuously grows, raising the global limits to unsafe levels? Arg in `SanityCheck`?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1949583127)
wording confusion: same peer change?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1949612609)
nit: would rather we check that we evicted the dos'y peer and somehow didn't *add* more resources allocated to him
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1949568939)
Maybe a bit too paranoid since it was just computed explicitly from the underlying list?
💬 yancyribbens commented on issue "wallet: Branch and Bound producing change":
(https://github.com/bitcoin/bitcoin/issues/31830#issuecomment-2649282877)
In otherwords, I think setting the change_fee to 359 would produce zero change, although I'm not sure what you are testing.
💬 brunoerg commented on issue "wallet: Branch and Bound producing change":
(https://github.com/bitcoin/bitcoin/issues/31830#issuecomment-2649284322)
This is a failing test to show BnB is producing change.
⚠️ darosior opened an issue: "`rpc_getblockstats.py` fails with `--gen-test-data`"
(https://github.com/bitcoin/bitcoin/issues/31838)
The test only pass with the currently generated data. It will fail when using the integrated feature to generate this data and running against it.

Reproduction instructions. On top of current master 1d813e4bf52a207ec526303df7b2e3d40d5eaa54 create a clean default build and run the test:
```
cmake -B repro
cmake --build repro -j20
./repro/test/functional/rpc_getblockstats.py --gen-test-data
```

Initially it will fail because it's missing the wallet arguments. Patch it with:
```diff
diff --git a/
...