🚀 fanquake merged a pull request: "ci: Switch to gcr.io mirror to avoid rate limits"
(https://github.com/bitcoin/bitcoin/pull/31906)
(https://github.com/bitcoin/bitcoin/pull/31906)
💬 fanquake commented on pull request "doc: add missing copyright headers":
(https://github.com/bitcoin/bitcoin/pull/31864#discussion_r1964160825)
I have changes for this locally, but the value is low, and ideally all of this code will soon be removed.
(https://github.com/bitcoin/bitcoin/pull/31864#discussion_r1964160825)
I have changes for this locally, but the value is low, and ideally all of this code will soon be removed.
💬 hebasto commented on pull request "build: Enhance Ccache performance across worktrees and build trees":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2672379835)
> I do understand that setting `base_dir` to `CMAKE_SOURCE_DIR` will not work if developers are placing their build directories completely outside of their source directories, or at inconsistent depths within their source directories, but this seems much less common and not really a consideration if we are just deciding on a default base_dir value.
Right. As for the PR author, it was not clear to me which cases are more common and which are less common, so I tried to take a more general appro
...
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2672379835)
> I do understand that setting `base_dir` to `CMAKE_SOURCE_DIR` will not work if developers are placing their build directories completely outside of their source directories, or at inconsistent depths within their source directories, but this seems much less common and not really a consideration if we are just deciding on a default base_dir value.
Right. As for the PR author, it was not clear to me which cases are more common and which are less common, so I tried to take a more general appro
...
💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2672397183)
> Code review ACK [879569c](https://github.com/bitcoin/bitcoin/commit/879569cab4e5b400350f3b95d7bee71b49636591) (no changes since last review other than rebase), but [again](https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2631914588) I think this PR should be prefixed with [BREAKING] or [INCOMPATIBLE] or [!] or ‼️ or ⚠️ and have a clear message to developers about what it does like "**Warning**: This PR changes build locations of newly built executables like `bitcoind` and `test_bitco
...
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2672397183)
> Code review ACK [879569c](https://github.com/bitcoin/bitcoin/commit/879569cab4e5b400350f3b95d7bee71b49636591) (no changes since last review other than rebase), but [again](https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2631914588) I think this PR should be prefixed with [BREAKING] or [INCOMPATIBLE] or [!] or ‼️ or ⚠️ and have a clear message to developers about what it does like "**Warning**: This PR changes build locations of newly built executables like `bitcoind` and `test_bitco
...
✅ fanquake closed a pull request: "Silent payment index (for light wallets and consistency check)"
(https://github.com/bitcoin/bitcoin/pull/28241)
(https://github.com/bitcoin/bitcoin/pull/28241)
💬 fanquake commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2672407221)
Closing this for now, given after a year and a half it's still a draft, based on a draft, based on another PR that is still being worked on. Maybe a new PR can be opened in future.
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2672407221)
Closing this for now, given after a year and a half it's still a draft, based on a draft, based on another PR that is still being worked on. Maybe a new PR can be opened in future.
💬 jsarenik commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2672409040)
How will the default signet directory name be here with this patch?
Could it possibly stay `signet` for the default [Signet](https://en.bitcoin.it/wiki/Signet)? I think the advantages of one default Signet are huge, one of them being the fact you can use [mempool.space/signet](https://mempool.space/signet). There is now a working faucet at [alt.signetfaucet.com](https://alt.signetfaucet.com/).
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2672409040)
How will the default signet directory name be here with this patch?
Could it possibly stay `signet` for the default [Signet](https://en.bitcoin.it/wiki/Signet)? I think the advantages of one default Signet are huge, one of them being the fact you can use [mempool.space/signet](https://mempool.space/signet). There is now a working faucet at [alt.signetfaucet.com](https://alt.signetfaucet.com/).
💬 1440000bytes commented on pull request "Implement OP_CHECKSIGFROMSTACK(VERIFY)":
(https://github.com/bitcoin/bitcoin/pull/29270#issuecomment-2672409877)
Quoting a line from a recent post written by AJ as a soft forking guide:
> Open a PR for the changes to core, along with thorough tests
https://ajtowns.github.io/bfg/finalization.html
(https://github.com/bitcoin/bitcoin/pull/29270#issuecomment-2672409877)
Quoting a line from a recent post written by AJ as a soft forking guide:
> Open a PR for the changes to core, along with thorough tests
https://ajtowns.github.io/bfg/finalization.html
✅ fanquake closed a pull request: "Implement OP_CHECKTEMPLATEVERIFY"
(https://github.com/bitcoin/bitcoin/pull/29280)
(https://github.com/bitcoin/bitcoin/pull/29280)
💬 fanquake commented on pull request "Implement OP_CHECKTEMPLATEVERIFY":
(https://github.com/bitcoin/bitcoin/pull/29280#issuecomment-2672416455)
Closing for now, given https://github.com/bitcoin/bitcoin/pull/29198 was closed.
(https://github.com/bitcoin/bitcoin/pull/29280#issuecomment-2672416455)
Closing for now, given https://github.com/bitcoin/bitcoin/pull/29198 was closed.
📝 instagibbs opened a pull request: "fuzz: add basic TxOrphanage::EraseForBlock cov"
(https://github.com/bitcoin/bitcoin/pull/31918)
Currently uncovered
(https://github.com/bitcoin/bitcoin/pull/31918)
Currently uncovered
💬 fanquake commented on pull request "fuzz: Mutate -max_len= during generation phase":
(https://github.com/bitcoin/bitcoin/pull/30371#issuecomment-2672445780)
> Turning into draft for now, to allow for more time to benchmark and evaluate this.
Any more thoughts on this?
(https://github.com/bitcoin/bitcoin/pull/30371#issuecomment-2672445780)
> Turning into draft for now, to allow for more time to benchmark and evaluate this.
Any more thoughts on this?
✅ maflcko closed a pull request: "fuzz: Mutate -max_len= during generation phase"
(https://github.com/bitcoin/bitcoin/pull/30371)
(https://github.com/bitcoin/bitcoin/pull/30371)
💬 fanquake commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#issuecomment-2672448003)
Is this still meant to be a draft?
(https://github.com/bitcoin/bitcoin/pull/31787#issuecomment-2672448003)
Is this still meant to be a draft?
💬 i-am-yuvi commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2672450161)
@jsarenik read the PR description https://github.com/bitcoin/bitcoin/pull/29838#issue-2233837348
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2672450161)
@jsarenik read the PR description https://github.com/bitcoin/bitcoin/pull/29838#issue-2233837348
✅ willcl-ark closed a pull request: "[WIP] net: return result from addnode RPC"
(https://github.com/bitcoin/bitcoin/pull/30381)
(https://github.com/bitcoin/bitcoin/pull/30381)
💬 willcl-ark commented on pull request "[WIP] net: return result from addnode RPC":
(https://github.com/bitcoin/bitcoin/pull/30381#issuecomment-2672463328)
Closing for now until I can circle back.
(https://github.com/bitcoin/bitcoin/pull/30381#issuecomment-2672463328)
Closing for now until I can circle back.
💬 maflcko commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#issuecomment-2672476623)
For reference, I can't review this, because I don't know if all the bash refactors/behavior changes here are correct and best practise, or if they could fail (and when) on error or on ancient versions of bash used in macOS.
(https://github.com/bitcoin/bitcoin/pull/31787#issuecomment-2672476623)
For reference, I can't review this, because I don't know if all the bash refactors/behavior changes here are correct and best practise, or if they could fail (and when) on error or on ancient versions of bash used in macOS.
💬 achow101 commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#issuecomment-2672488147)
ACK ca6aa0b9bee3fdf355b7154a9a686a80977f2a02
(https://github.com/bitcoin/bitcoin/pull/30302#issuecomment-2672488147)
ACK ca6aa0b9bee3fdf355b7154a9a686a80977f2a02
💬 i-am-yuvi commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r1964227405)
It might be helpful to consider giving a `Concept ACK` or `NACK` before the review. I came across this idea [here](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review).
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r1964227405)
It might be helpful to consider giving a `Concept ACK` or `NACK` before the review. I came across this idea [here](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review).