π¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550483747)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550483747)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
π¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550483813)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550483813)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
π¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550483869)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550483869)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
π¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550483950)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550483950)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
π¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484078)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484078)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
π¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484161)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484161)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
π¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484288)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484288)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
π¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484381)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484381)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
π¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484520)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
(https://github.com/bitcoin/bitcoin/pull/32958#discussion_r2550484520)
Thanks, updated in 4e038c4b7a6fbfaafe1f2570592e2bb751f9be09
π¬ ismaelsadeeq commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3564018931)
> It seems counter intuitive, but from a memory management perspective IPC clients are treated no different than our own code. And if we started FIFO deleting templates that are used by our own code, we'd crash.
IMHO I think we should separate that, and treat clients differently from our own code, because they are different codebases and separate applications with their own memory.
> Maybe there are 100 downstream ASICs, one of which is very slow at loading templates, so itβs only given a
...
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3564018931)
> It seems counter intuitive, but from a memory management perspective IPC clients are treated no different than our own code. And if we started FIFO deleting templates that are used by our own code, we'd crash.
IMHO I think we should separate that, and treat clients differently from our own code, because they are different codebases and separate applications with their own memory.
> Maybe there are 100 downstream ASICs, one of which is very slow at loading templates, so itβs only given a
...
π€ brunoerg reviewed a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3493836797)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3493836797)
Concept ACK
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550626324)
Retouched a little bit, to use `extract()`, somewhat differently.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550626324)
Retouched a little bit, to use `extract()`, somewhat differently.
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3564170142)
`e9b436a408...7987000d36`: take some suggestions
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3564170142)
`e9b436a408...7987000d36`: take some suggestions
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550639055)
Yes, hmm... what would be the TODO text? There is nothing to-do around there - the assigned string to `what` is the correct one. Whenever a caller starts calling `CWallet::SubmitTxMemoryPoolAndRelay()` with `broadcast_method` equal to `NO_MEMPOOL_PRIVATE_BROADCAST`, then `CWallet::SubmitTxMemoryPoolAndRelay()` will just work without having to change anything in it.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550639055)
Yes, hmm... what would be the TODO text? There is nothing to-do around there - the assigned string to `what` is the correct one. Whenever a caller starts calling `CWallet::SubmitTxMemoryPoolAndRelay()` with `broadcast_method` equal to `NO_MEMPOOL_PRIVATE_BROADCAST`, then `CWallet::SubmitTxMemoryPoolAndRelay()` will just work without having to change anything in it.
π¬ l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550662202)
Shouldn't the logic be added when it's actually used? Otherwise it's just dead code waiting for event.
An explicit throw here would clarify that this is WIP.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550662202)
Shouldn't the logic be added when it's actually used? Otherwise it's just dead code waiting for event.
An explicit throw here would clarify that this is WIP.
π¬ l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550717279)
Unfortunately it seems we're stuck at
https://github.com/bitcoin/bitcoin/blob/fadad7a49477cd61fbbfe20a0a61023c2d4d70a1/depends/hosts/darwin.mk#L3
where the C++20 spec isn't fully implemented yet.
For proper spaceship operator support we would need: https://developer.apple.com/documentation/xcode-release-notes/xcode-16_3-release-notes:
> The following C++20 features have been implemented:
> The Mothership has Landed: Adding <=> to the Library ([P1614R2](https://wg21.link/P1614R2))
> Symmetr
...
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550717279)
Unfortunately it seems we're stuck at
https://github.com/bitcoin/bitcoin/blob/fadad7a49477cd61fbbfe20a0a61023c2d4d70a1/depends/hosts/darwin.mk#L3
where the C++20 spec isn't fully implemented yet.
For proper spaceship operator support we would need: https://developer.apple.com/documentation/xcode-release-notes/xcode-16_3-release-notes:
> The following C++20 features have been implemented:
> The Mothership has Landed: Adding <=> to the Library ([P1614R2](https://wg21.link/P1614R2))
> Symmetr
...
π¬ l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550720411)
> comment/note that now the declaration order is now relevant
Instead of a dead comment, can we add a test case that will instead fail the build when somebody accidentally changes this?
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550720411)
> comment/note that now the declaration order is now relevant
Instead of a dead comment, can we add a test case that will instead fail the build when somebody accidentally changes this?
π¬ romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2550802960)
Sounds good - fixed in [e14650967d..8457a71a64](https://github.com/bitcoin/bitcoin/compare/e14650967dc95da5c10e0d6183b6eac3e8243fe5..8457a71a6420792a1b9c7c8645c9e5c607a4b90c).
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2550802960)
Sounds good - fixed in [e14650967d..8457a71a64](https://github.com/bitcoin/bitcoin/compare/e14650967dc95da5c10e0d6183b6eac3e8243fe5..8457a71a6420792a1b9c7c8645c9e5c607a4b90c).
π¬ romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2550803976)
Many thanks! [e14650967d..8457a71a64](https://github.com/bitcoin/bitcoin/compare/e14650967dc95da5c10e0d6183b6eac3e8243fe5..8457a71a6420792a1b9c7c8645c9e5c607a4b90c)
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2550803976)
Many thanks! [e14650967d..8457a71a64](https://github.com/bitcoin/bitcoin/compare/e14650967dc95da5c10e0d6183b6eac3e8243fe5..8457a71a6420792a1b9c7c8645c9e5c607a4b90c)
π¬ romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2550804495)
Fixed by [e14650967d..8457a71a64](https://github.com/bitcoin/bitcoin/compare/e14650967dc95da5c10e0d6183b6eac3e8243fe5..8457a71a6420792a1b9c7c8645c9e5c607a4b90c).
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2550804495)
Fixed by [e14650967d..8457a71a64](https://github.com/bitcoin/bitcoin/compare/e14650967dc95da5c10e0d6183b6eac3e8243fe5..8457a71a6420792a1b9c7c8645c9e5c607a4b90c).