Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 pinheadmz commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#discussion_r2167236137)
Thanks I'll take this suggestion. My only concern is that technically there could be a future regression where a user setting timeout to 1 second ends up lasting 60 seconds... I'll add `timeout=5` for a reasonable clamp.
🤔 pinheadmz reviewed a pull request: "rpc: use CScheduler for HTTPRPCTimer"
(https://github.com/bitcoin/bitcoin/pull/32796#pullrequestreview-2958751819)
Rebase to include suggestions from @fjahr and @vasild thanks!!
💬 pinheadmz commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#discussion_r2167221436)
Thanks I'll remove reference to libevent (thats the whole point anyway!) and explain why there is a margin of error in the test
💬 pinheadmz commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#discussion_r2167060793)
👍
💬 pinheadmz commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#discussion_r2167216427)
Good feedback I think you're right, its cleaner to just store a reference to the scheduler here. It requires some weird pointer / `any` juggling in`StartHTTPRPC()` -- to keep the `std::any` abstraction
💬 Zeegaths commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2167432256)
if i run this its goes to version 30, which does not exist yet..,
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3005901522)
> together staying just below the global announcement and usage limits, but massively exceeding the per-peer usage reservation

Whiteboarded in person. Hopefully this explanation is clear and explains it in a more permanent location. If this ends up being correct we can adapt the benchmark to be more realistic.

The attack is as follows, assuming a global announcement count of 24,000, and per-peer reservation limit of 404kWU:

1) Peer 0 sends a 260WU orphan
2) Peers 1 through 124 announce
...
💬 willcl-ark commented on pull request "doc: Add fetching single PRs from upstream to productivity.md":
(https://github.com/bitcoin/bitcoin/pull/32783#discussion_r2167461391)
Thanks, fixed in 45b1d39757668939b03b27401c324a938ef0cd8d
🤔 janb84 reviewed a pull request: "doc: Add fetching single PRs from upstream to productivity.md"
(https://github.com/bitcoin/bitcoin/pull/32783#pullrequestreview-2959405149)
re ACK 45b1d39757668939b03b27401c324a938ef0cd8d

Changes sinds last ACK:
- Small textual changes / fixed typo's
💬 luke-jr commented on pull request "rpc, doc: clarify the response of listtransactions RPC":
(https://github.com/bitcoin/bitcoin/pull/32737#discussion_r2167466882)
"output" is kind of a low-level detail, while this is a high-level RPC. It's better to think of entries as logical/financial transactions, as opposed to blockchain transactions.
💬 stringintech commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#discussion_r2167480268)
I tried this and it seems that doing this instead of restarting the node does not reset the peer indexes, and I can no longer count on the peer indexes in the debug logs.
For example, for the `test_normal_peer_timeout` subset:
```
Timeout downloading headers, disconnecting peer=0
```
becomes
```
Timeout downloading headers, disconnecting peer=3
```
Although still having `peer=0` for the `test_noban_peer_timeout` subtest since we need a restart for that. But it shouldn't matter if you t
...
💬 pablomartin4btc commented on pull request "doc: Add fetching single PRs from upstream to productivity.md":
(https://github.com/bitcoin/bitcoin/pull/32783#issuecomment-3005968667)
re-ACK 45b1d39
🤔 glozow reviewed a pull request: "doc: add release notes for #32425"
(https://github.com/bitcoin/bitcoin/pull/32727#pullrequestreview-2959472194)
ACK b9a2e8ee965daffe2bc618f58b21f0ddebadeb23, thanks for taking suggestions!
🚀 glozow merged a pull request: "doc: add release notes for #32425"
(https://github.com/bitcoin/bitcoin/pull/32727)
🚀 glozow merged a pull request: "wallet: Remove `CWalletTx::fTimeReceivedIsTxTime`"
(https://github.com/bitcoin/bitcoin/pull/32768)
🚀 glozow merged a pull request: "wallet: Always set descriptor cache upgraded flag for new wallets"
(https://github.com/bitcoin/bitcoin/pull/32597)
💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2167557225)
It was `--target-dir`. Applied suggestion.
💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2167557715)
Done.
💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2167558515)
Sure, done.
💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2167558872)
Done.