β οΈ 0xB10C opened an issue: "Can't copy to 'build-aux/config.guess' in autoconf.sh: Permission denied"
(https://github.com/bitcoin/bitcoin/issues/27873)
https://github.com/bitcoin/bitcoin/issues/26422 broke the `autogen.sh` part of my development builds on NixOS - found #26422 by bisecting. The error is:
```
$ ./autogen.sh
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'build-aux/m4'.
libtoolize: copying file 'build-aux/m4/libtool.m4'
libtoolize: copying file 'build-aux/m4/ltoptions.m4'
libtoolize: copying file 'bu
...
(https://github.com/bitcoin/bitcoin/issues/27873)
https://github.com/bitcoin/bitcoin/issues/26422 broke the `autogen.sh` part of my development builds on NixOS - found #26422 by bisecting. The error is:
```
$ ./autogen.sh
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'build-aux/m4'.
libtoolize: copying file 'build-aux/m4/libtool.m4'
libtoolize: copying file 'build-aux/m4/ltoptions.m4'
libtoolize: copying file 'bu
...
π¬ 0xB10C commented on issue "Can't copy to 'build-aux/config.guess' in autoconf.sh: Permission denied":
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589157862)
https://github.com/bitcoin/bitcoin/blob/8de9bb7a5ab04669369e2bb59ea92a5c1a91a8d2/autogen.sh#L18 evaluates to true for me. The `depends/config.guess` file has a timestamp of `2023-01-01` while the generated `build-aux/config.guess` file has a timestamp of `2021-06-03`.
The generated `build-aux/config.guess` file has the file permissions `-r-xr-xr-x` so overwriting it fails.
```
$ autoreconf --version
autoreconf (GNU Autoconf) 2.71
```
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589157862)
https://github.com/bitcoin/bitcoin/blob/8de9bb7a5ab04669369e2bb59ea92a5c1a91a8d2/autogen.sh#L18 evaluates to true for me. The `depends/config.guess` file has a timestamp of `2023-01-01` while the generated `build-aux/config.guess` file has a timestamp of `2021-06-03`.
The generated `build-aux/config.guess` file has the file permissions `-r-xr-xr-x` so overwriting it fails.
```
$ autoreconf --version
autoreconf (GNU Autoconf) 2.71
```
π¬ 0xB10C commented on issue "Can't copy to 'build-aux/config.guess' in autoconf.sh: Permission denied":
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589198065)
> The generated `build-aux/config.guess` file has the file permissions `-r-xr-xr-x` so overwriting it fails.
`autoreconf --install --force --warnings=all` copies from the nix-store (in this case `/nix/store/3bvdlza5sxy6kjs9y7v0kbx4gxgp0wg8-autoconf-2.71/share/autoconf/build-aux/config.guess`) to `build-aux/config.guess`. The file has `-r-xr-xr-x` permissions in the nix-store. These permissions are preserved when copying. So the problem is a combination of how the nix-store works, how `autorec
...
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589198065)
> The generated `build-aux/config.guess` file has the file permissions `-r-xr-xr-x` so overwriting it fails.
`autoreconf --install --force --warnings=all` copies from the nix-store (in this case `/nix/store/3bvdlza5sxy6kjs9y7v0kbx4gxgp0wg8-autoconf-2.71/share/autoconf/build-aux/config.guess`) to `build-aux/config.guess`. The file has `-r-xr-xr-x` permissions in the nix-store. These permissions are preserved when copying. So the problem is a combination of how the nix-store works, how `autorec
...
π¬ 0xB10C commented on issue "Can't copy to 'build-aux/config.guess' in autoconf.sh: Permission denied":
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589220664)
Changing the permissions for build-aux/config.{guess, sub} with `chmod ug+w` allows us to overwrite the files. Will PR this.
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589220664)
Changing the permissions for build-aux/config.{guess, sub} with `chmod ug+w` allows us to overwrite the files. Will PR this.
π¬ willcl-ark commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228062756)
nit:
```suggestion
// How often to flush fee estimates to disk.
```
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228062756)
nit:
```suggestion
// How often to flush fee estimates to disk.
```
π¬ willcl-ark commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228063658)
nit: Perhaps we can give this a slightly more descriptive name? `FEE_ESTIMATE_MAX_AGE` or something?
Would need to update comment below if you do change
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228063658)
nit: Perhaps we can give this a slightly more descriptive name? `FEE_ESTIMATE_MAX_AGE` or something?
Would need to update comment below if you do change
π¬ fanquake commented on pull request "build: suppress external warnings by default":
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589239464)
> IIRC this is only needed for bdb4 and qt5,
There are likely warnings produced for all dependencies (includeing Boost and libevent),
because warning output is dependant on:
* architecture
* operating system (& version)
* compiler (& version)
* compiler flags being used
* other compile options i.e LTO
* packages being used (& version) (& system vs depends)
Where a number of the above continue to change over time.
> so someone compiling bitcoind with sqlite shouldn't have to sup
...
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589239464)
> IIRC this is only needed for bdb4 and qt5,
There are likely warnings produced for all dependencies (includeing Boost and libevent),
because warning output is dependant on:
* architecture
* operating system (& version)
* compiler (& version)
* compiler flags being used
* other compile options i.e LTO
* packages being used (& version) (& system vs depends)
Where a number of the above continue to change over time.
> so someone compiling bitcoind with sqlite shouldn't have to sup
...
π 0xB10C opened a pull request: "build: make sure we can overwrite config.{guess,sub} before doing so"
(https://github.com/bitcoin/bitcoin/pull/27875)
Since ea7b8528 (#26422), `autogen.sh` overwrites the `build-aux/config.{guess, sub}` files (installed there by `autoreconf`) with the `depends/config.{guess, sub}` files if these are newer.
The `autoreconf` tool copies them from it's `share/autoconf/build-aux/` directory. Specifically on NixOS, the `share/autoconf/build-aux/` files are located in the nix-store and are read-only. `autoreconf` preserves the read-only permissions when copying. Overwriting them with our `depends/config.{guess, su
...
(https://github.com/bitcoin/bitcoin/pull/27875)
Since ea7b8528 (#26422), `autogen.sh` overwrites the `build-aux/config.{guess, sub}` files (installed there by `autoreconf`) with the `depends/config.{guess, sub}` files if these are newer.
The `autoreconf` tool copies them from it's `share/autoconf/build-aux/` directory. Specifically on NixOS, the `share/autoconf/build-aux/` files are located in the nix-store and are read-only. `autoreconf` preserves the read-only permissions when copying. Overwriting them with our `depends/config.{guess, su
...
π¬ hebasto commented on pull request "build: make sure we can overwrite config.{guess,sub} before doing so":
(https://github.com/bitcoin/bitcoin/pull/27875#issuecomment-1589290446)
Concept ACK. However, I'm not a NixOS user and cannot test this PR, unfortunately.
(https://github.com/bitcoin/bitcoin/pull/27875#issuecomment-1589290446)
Concept ACK. However, I'm not a NixOS user and cannot test this PR, unfortunately.
π¬ Xekyo commented on pull request "[coinselection] Increase SRD target by change_fee":
(https://github.com/bitcoin/bitcoin/pull/27846#issuecomment-1589306983)
I cannot reproduce the issue with `test/functional/interface_usdt_mempool.py` locally. Any idea whatβs up here?
(https://github.com/bitcoin/bitcoin/pull/27846#issuecomment-1589306983)
I cannot reproduce the issue with `test/functional/interface_usdt_mempool.py` locally. Any idea whatβs up here?
π TheCharlatan opened a pull request: "refactor(test): Use datadir from options in chainstatemanager test"
(https://github.com/bitcoin/bitcoin/pull/27876)
This should make the test less reliant on argument state from the test setup. This is a follow-up PR as requested in https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1224638890.
(https://github.com/bitcoin/bitcoin/pull/27876)
This should make the test less reliant on argument state from the test setup. This is a follow-up PR as requested in https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1224638890.
π¬ TheCharlatan commented on pull request "kernel: Remove args, settings, chainparams, chainparamsbase from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1228126265)
https://github.com/bitcoin/bitcoin/pull/27876
(https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1228126265)
https://github.com/bitcoin/bitcoin/pull/27876
π€ willcl-ark reviewed a pull request: "Mempool: persist mempoolminfee accross restarts"
(https://github.com/bitcoin/bitcoin/pull/27859#pullrequestreview-1477104823)
I like the approach here and think persisting mempool min fee is useful. I also agree that re-using fee_estimates.dat would make sense for it, as it likely doesn't need its own file.
Left a few nits and suggestions inline.
(https://github.com/bitcoin/bitcoin/pull/27859#pullrequestreview-1477104823)
I like the approach here and think persisting mempool min fee is useful. I also agree that re-using fee_estimates.dat would make sense for it, as it likely doesn't need its own file.
Left a few nits and suggestions inline.
π¬ willcl-ark commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228086493)
An alternative could be to also set `rollingMinimumFeeRate`, and then also set`lastRollingFeeUpdate` to the timestamp of the min fee dump, and then just let `GetMinFee() run its exponential backoff algorithm as per normal?
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228086493)
An alternative could be to also set `rollingMinimumFeeRate`, and then also set`lastRollingFeeUpdate` to the timestamp of the min fee dump, and then just let `GetMinFee() run its exponential backoff algorithm as per normal?
π¬ willcl-ark commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228088807)
nit: same here re. comments (and a few more places).
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228088807)
nit: same here re. comments (and a few more places).
π¬ willcl-ark commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228088500)
nit: I think the code is clear enough here that comments aren't really needed?
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228088500)
nit: I think the code is clear enough here that comments aren't really needed?
π¬ willcl-ark commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228094327)
I would agree that it makes sense to dump into fee_estimates.dat.
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228094327)
I would agree that it makes sense to dump into fee_estimates.dat.
π¬ instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1228135411)
I don't understand what this scenario has to do with this case. A will be accepted, then B rejected, then B+C rejected, all for having package feerates lower than 10 sat/vbyte.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1228135411)
I don't understand what this scenario has to do with this case. A will be accepted, then B rejected, then B+C rejected, all for having package feerates lower than 10 sat/vbyte.
π MarcoFalke approved a pull request: "test: (refactor) Use datadir from options in chainstatemanager test"
(https://github.com/bitcoin/bitcoin/pull/27876#pullrequestreview-1477197363)
lgtm ACK d54819d74e04e6105c1f0362755f5bcfa845eefd
(https://github.com/bitcoin/bitcoin/pull/27876#pullrequestreview-1477197363)
lgtm ACK d54819d74e04e6105c1f0362755f5bcfa845eefd
π¬ MarcoFalke commented on pull request "test: (refactor) Use datadir from options in chainstatemanager test":
(https://github.com/bitcoin/bitcoin/pull/27876#discussion_r1228144723)
review note, this creates a copy and is not a reference, thus safe
(https://github.com/bitcoin/bitcoin/pull/27876#discussion_r1228144723)
review note, this creates a copy and is not a reference, thus safe
π¬ glozow commented on pull request "wallet: Give deprecation warning when loading a legacy wallet":
(https://github.com/bitcoin/bitcoin/pull/27869#issuecomment-1589349516)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27869#issuecomment-1589349516)
Concept ACK