Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114084831)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

might as well use and assert return of `CheckNumEvictions`
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114089251)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

nit: s/peer0/dos_peer/
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114080543)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

could run these checks just before additions to assert no-op
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114103109)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

`EraseForPeer` and `EraseTx` coverage here would be good. IIUC max memory goes up IFF a peer has a live orphan announcement?
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114084722)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

might as well use and assert return of `CheckNumEvictions`
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114085090)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

might as well use and assert return of `CheckNumEvictions`
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114091408)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a
```Suggestion
BOOST_CHECK_EQUAL(orphanage.AnnouncementsFromPeer(peer1), 0);
BOOST_CHECK_EQUAL(orphanage.AnnouncementsFromPeer(peer2), 0);
```
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114104811)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

Single scenario that boots out 2+ txns with a single `CheckNumEvictions` seems apt
💬 fanquake commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2919737881)
Actually, we can now use 0.16.6: https://github.com/lief-project/LIEF/releases/tag/0.16.6, which includes that change.
💬 ryanofsky commented on pull request "Mining: Avoid copying template CBlocks":
(https://github.com/bitcoin/bitcoin/pull/32547#issuecomment-2919753784)
> The constness of methods is a property of the interface. It is a way to say calling it will not change the object

Yes, I understand the intent of marking `interfaces::BlockTemplate` methods `const` is to say calling those methods will not change the object. And I agree using `const` to prevent changing the object would be a nice goal, but unfortunately that's not what `const` keyword does here.

Marking the methods `const` does not mark the block template data `const` or do anything that
...
📝 fanquake converted_to_draft a pull request: "Mining: Avoid copying template CBlocks"
(https://github.com/bitcoin/bitcoin/pull/32547)
Refactoring to avoid unnecessary copies/complexity, at least in the mainnet paths.

Also abstracts out a new `TemplateToJSON` function to keep track of variable scoping better.
💬 fanquake commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2919777106)
Backported to `27.x` in #32479.
💬 fanquake commented on pull request "[28.x] Backport guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32639#issuecomment-2919815304)
Guix Build:
```bash
f68d83b36bec27fd42c754af357d205c9abf7e2e90b69e7d1f9136813a191745 guix-build-5c2ba9f583e2/output/aarch64-linux-gnu/SHA256SUMS.part
e29852e1e6905d2c90734c39cf2994680b83228148342e25b0f44b44a6962a1a guix-build-5c2ba9f583e2/output/aarch64-linux-gnu/bitcoin-5c2ba9f583e2-aarch64-linux-gnu-debug.tar.gz
949df5bf91e782332065362d02b77ea8f2686433eb461e2458b91996cf97cd8f guix-build-5c2ba9f583e2/output/aarch64-linux-gnu/bitcoin-5c2ba9f583e2-aarch64-linux-gnu.tar.gz
eb18222f14ad9cec
...
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114242408)
what if the peer sent us the first non-reconsidered orphan? would that not be considered below due to sequence number being 0?
💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2114255993)
I have benchmarked both approaches (reading block undo data directly from storage and returning re-serialized spent txouts) using https://github.com/bitcoin/bitcoin/pull/32540/commits/d5828eadc39772bf825eb4141340a281f4a490a7 with https://github.com/romanz/bench-rest/commit/f90aaddf1c3a0c0687f373cc89b0c343e35df739:

## Fetching block undo data (using `ReadRawBlockUndo`)
```
$ time cargo run --release --bin blockundo
Finished `release` profile [optimized] target(s) in 0.02s
Running
...
fanquake closed an issue: "Bug in PartiallyDownloadedBlock fuzz test logic or behavior"
(https://github.com/bitcoin/bitcoin/issues/32640)
💬 achow101 commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2114271026)
Yes, see `verbose|verbosity` in `getblock` for example.
💬 fanquake commented on issue "Assertion failed: TxStateSerializedIndex(wtx.m_state) == TxStateSerializedIndex(state) (wallet/wallet.cpp: AddToWallet: 1094)":
(https://github.com/bitcoin/bitcoin/issues/32625#issuecomment-2919878321)
> This was fixed in v29, specifically in https://github.com/bitcoin/bitcoin/pull/31757.

Is that the right PR? Those changes are not in the 29.x branch.
💬 luke-jr commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-2919900895)
I'm not sure it makes sense to adjust this based on dbcache size. Won't a given batch size use the same amount of memory regardless of the size of the dbcache?
💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-2919920634)
The tidy job is failing because it doesn't like the logging jobs used in lambdas. It seems like this is pre-existing so I'm not sure why it's failing now. The windows cross-built job is failing on the `rate_limiting` test at every file-size comparison. I don't have a windows machine to debug this, but I think maybe `fs::file_size` is failing or some other quirk is showing up?