✅ laanwj closed a pull request: "[RFC] dbwrapper: Set global leveldb mmap limit"
(https://github.com/bitcoin/bitcoin/pull/32447)
(https://github.com/bitcoin/bitcoin/pull/32447)
💬 purpleKarrot commented on pull request "depends: Avoid using helper variables in toolchain file":
(https://github.com/bitcoin/bitcoin/pull/31360#issuecomment-2862686579)
Ack 7343a1846cebf74ffef3c54e05b633df629510a1
I am not particularly happy with logic in toolchain files. Removing helper variables is a step in the right direction.
(https://github.com/bitcoin/bitcoin/pull/31360#issuecomment-2862686579)
Ack 7343a1846cebf74ffef3c54e05b633df629510a1
I am not particularly happy with logic in toolchain files. Removing helper variables is a step in the right direction.
💬 laanwj commented on pull request "[RFC] dbwrapper: Set global leveldb mmap limit":
(https://github.com/bitcoin/bitcoin/pull/32447#issuecomment-2862713589)
See bitcoin-core/leveldb-subtree#52 instead.
(https://github.com/bitcoin/bitcoin/pull/32447#issuecomment-2862713589)
See bitcoin-core/leveldb-subtree#52 instead.
🤔 janb84 reviewed a pull request: "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory"
(https://github.com/bitcoin/bitcoin/pull/32423#pullrequestreview-2824869529)
tACK [3acfc07](https://github.com/bitcoin/bitcoin/commits/3acfc071c3445e943069b2778bbc5c74f702ef36)
- code review ✅
- compiled, run all tests ✅
- tested on regtest with user/pass & did remote curl call to test rpc user&pass func. ✅
(https://github.com/bitcoin/bitcoin/pull/32423#pullrequestreview-2824869529)
tACK [3acfc07](https://github.com/bitcoin/bitcoin/commits/3acfc071c3445e943069b2778bbc5c74f702ef36)
- code review ✅
- compiled, run all tests ✅
- tested on regtest with user/pass & did remote curl call to test rpc user&pass func. ✅
💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862815509)
The feedback from @fanquake has been addressed.
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862815509)
The feedback from @fanquake has been addressed.
💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2079588558)
> I think for now we could just assert `binary.resources_manager.has_manifest`, if we care that this is being added? Could save anything else for if/when it's actually being used?
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862815509).
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2079588558)
> I think for now we could just assert `binary.resources_manager.has_manifest`, if we care that this is being added? Could save anything else for if/when it's actually being used?
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862815509).
💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2079589157)
> > What alternatives are you suggesting for configuring the Developer Command Prompt?
>
> Directly running the command that somebody who owns a Windows computer would run, that doesn't require installing NPM, and then executing somebody else's JavaScript.
Thanks. [Reworked](https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862815509).
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2079589157)
> > What alternatives are you suggesting for configuring the Developer Command Prompt?
>
> Directly running the command that somebody who owns a Windows computer would run, that doesn't require installing NPM, and then executing somebody else's JavaScript.
Thanks. [Reworked](https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862815509).
💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2079591061)
I can add a note if I touch the PR again
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2079591061)
I can add a note if I touch the PR again
💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2079591354)
Leaving as-is for now
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2079591354)
Leaving as-is for now
👍 hebasto approved a pull request: "build: let CMake determine the year"
(https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2824945378)
re-ACK dfeac76b951fc2d28e0832b176f3c8ad595697f1.
(https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2824945378)
re-ACK dfeac76b951fc2d28e0832b176f3c8ad595697f1.
💬 janb84 commented on pull request "doc: swap "Docker image" for "container image"":
(https://github.com/bitcoin/bitcoin/pull/32444#issuecomment-2862868108)
ACK https://github.com/bitcoin/bitcoin/commit/1372eb09c5d031315bc6cde34f7a15297e96f4cc
(https://github.com/bitcoin/bitcoin/pull/32444#issuecomment-2862868108)
ACK https://github.com/bitcoin/bitcoin/commit/1372eb09c5d031315bc6cde34f7a15297e96f4cc
💬 hebasto commented on pull request "build: simplify *ifaddr handling":
(https://github.com/bitcoin/bitcoin/pull/32446#issuecomment-2862878161)
Concept ACK. I have considered a similar idea.
(https://github.com/bitcoin/bitcoin/pull/32446#issuecomment-2862878161)
Concept ACK. I have considered a similar idea.
💬 Sjors commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2862895879)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2862895879)
Concept ACK.
💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862902153)
My Guix build:
```
aarch64
be6aa004944a606e9bbef9129b971960186a533f6572c5fb2e522fabc544c611 guix-build-477b69ad19d7/output/dist-archive/bitcoin-477b69ad19d7.tar.gz
e75f1ed645abf5d086f5e328107ce804e3ebec9ac5b2d0d8a9d037744e2e4859 guix-build-477b69ad19d7/output/x86_64-w64-mingw32/SHA256SUMS.part
526d76c4402086b1f8382d5b461162485efcaaf1ebf98a23dc9fd8b8f7bd62d8 guix-build-477b69ad19d7/output/x86_64-w64-mingw32/bitcoin-477b69ad19d7-win64-codesigning.tar.gz
11029fcb9a8f11c7df44b2dfbda58392d22
...
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2862902153)
My Guix build:
```
aarch64
be6aa004944a606e9bbef9129b971960186a533f6572c5fb2e522fabc544c611 guix-build-477b69ad19d7/output/dist-archive/bitcoin-477b69ad19d7.tar.gz
e75f1ed645abf5d086f5e328107ce804e3ebec9ac5b2d0d8a9d037744e2e4859 guix-build-477b69ad19d7/output/x86_64-w64-mingw32/SHA256SUMS.part
526d76c4402086b1f8382d5b461162485efcaaf1ebf98a23dc9fd8b8f7bd62d8 guix-build-477b69ad19d7/output/x86_64-w64-mingw32/bitcoin-477b69ad19d7-win64-codesigning.tar.gz
11029fcb9a8f11c7df44b2dfbda58392d22
...
💬 fanquake commented on pull request "dbwrapper: Bump LevelDB max file size to 32 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2862903991)
Anyone that reviewed here, might be interested in https://github.com/bitcoin-core/leveldb-subtree/pull/52.
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2862903991)
Anyone that reviewed here, might be interested in https://github.com/bitcoin-core/leveldb-subtree/pull/52.
👍 laanwj approved a pull request: "build: let CMake determine the year"
(https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2824986604)
Code review ACK dfeac76b951fc2d28e0832b176f3c8ad595697f1
For example:
```sh
$ export SOURCE_DATE_EPOCH=1
$ cmake -B build
$ grep YEAR build/src/*.h
build/src/bitcoin-build-config.h:#define COPYRIGHT_YEAR 1970
```
In the guix build it sets `SOURCE_DATE_EPOCH` based on the last commit
```
SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH:-$(git -c log.showSignature=false log --format=%at -1)}"
```
So ostensibly this works out the correct date deterministically.
Should test an actual guix bui
...
(https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2824986604)
Code review ACK dfeac76b951fc2d28e0832b176f3c8ad595697f1
For example:
```sh
$ export SOURCE_DATE_EPOCH=1
$ cmake -B build
$ grep YEAR build/src/*.h
build/src/bitcoin-build-config.h:#define COPYRIGHT_YEAR 1970
```
In the guix build it sets `SOURCE_DATE_EPOCH` based on the last commit
```
SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH:-$(git -c log.showSignature=false log --format=%at -1)}"
```
So ostensibly this works out the correct date deterministically.
Should test an actual guix bui
...
👍 theStack approved a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2824981042)
re-ACK f09e1f943dbd1e5865dece3c35f342e4f0b31b9e :writing_hand: :hash:
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2824981042)
re-ACK f09e1f943dbd1e5865dece3c35f342e4f0b31b9e :writing_hand: :hash:
💬 theStack commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2079628809)
nitty nit: now that the checks are done via `testmempool` above, these two lines could be deleted
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2079628809)
nitty nit: now that the checks are done via `testmempool` above, these two lines could be deleted
💬 fanquake commented on pull request "doc: swap "Docker image" for "container image"":
(https://github.com/bitcoin/bitcoin/pull/32444#issuecomment-2862942029)
Pushed a commit up here: https://github.com/fanquake/core-review/commit/1656c4891dc1ac83b66765ba5a2607815b91d6eb.
(https://github.com/bitcoin/bitcoin/pull/32444#issuecomment-2862942029)
Pushed a commit up here: https://github.com/fanquake/core-review/commit/1656c4891dc1ac83b66765ba5a2607815b91d6eb.
🚀 fanquake merged a pull request: "doc: swap "Docker image" for "container image""
(https://github.com/bitcoin/bitcoin/pull/32444)
(https://github.com/bitcoin/bitcoin/pull/32444)