💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-3201656913)
I separate the refactoring commits (first four) into it's own PR in https://github.com/bitcoin/bitcoin/pull/33218
Hence putting this in draft.
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-3201656913)
I separate the refactoring commits (first four) into it's own PR in https://github.com/bitcoin/bitcoin/pull/33218
Hence putting this in draft.
📝 ismaelsadeeq converted_to_draft a pull request: "Fees: add Fee rate Forecaster Manager"
(https://github.com/bitcoin/bitcoin/pull/31664)
- This PR implements the core component of #30392, introducing a new fee rate forecasting module.
The primary addition is a `ForecasterManager` that coordinates multiple forecasting strategies to be able to provide better transaction fee rate predictions.
### Key Changes
1. **Refactoring of `CBlockPolicyEstimator`**
- Renames `fees` to `block_policy_estimator` for clarity.
- Renames`fees_args` to `block_policy_estimator_args` for clarity
- Renames `policy_fee_tests` t
...
(https://github.com/bitcoin/bitcoin/pull/31664)
- This PR implements the core component of #30392, introducing a new fee rate forecasting module.
The primary addition is a `ForecasterManager` that coordinates multiple forecasting strategies to be able to provide better transaction fee rate predictions.
### Key Changes
1. **Refactoring of `CBlockPolicyEstimator`**
- Renames `fees` to `block_policy_estimator` for clarity.
- Renames`fees_args` to `block_policy_estimator_args` for clarity
- Renames `policy_fee_tests` t
...
💬 polespinasa commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2285946865)
Maybe after https://github.com/bitcoin/bitcoin/pull/32750 can use CFeeRate to keep consistency with the rest of the code.
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2285946865)
Maybe after https://github.com/bitcoin/bitcoin/pull/32750 can use CFeeRate to keep consistency with the rest of the code.
💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2285951930)
Yeah makes sense.
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2285951930)
Yeah makes sense.
⚠️ hebasto opened an issue: "depends: `native_libmultiprocess` fails to build on OpenBSD"
(https://github.com/bitcoin/bitcoin/issues/33219)
The `native_libmultiprocess` package fails to build on OpenBSD starting with #32345:
```
$ uname -a
OpenBSD openbsd.home 7.7 GENERIC.MP#625 amd64
$ gmake -j 1 -C depends/ MULTIPROCESS=1 native_libmultiprocess
gmake: Entering directory '/home/hebasto/dev/bitcoin/depends'
Extracting native_libmultiprocess...
(SHA256) /home/hebasto/dev/bitcoin/depends/sources/src-ipc-libmultiprocess.tar: OK
Preprocessing native_libmultiprocess...
Configuring native_libmultiprocess...
-- The CXX compiler identificat
...
(https://github.com/bitcoin/bitcoin/issues/33219)
The `native_libmultiprocess` package fails to build on OpenBSD starting with #32345:
```
$ uname -a
OpenBSD openbsd.home 7.7 GENERIC.MP#625 amd64
$ gmake -j 1 -C depends/ MULTIPROCESS=1 native_libmultiprocess
gmake: Entering directory '/home/hebasto/dev/bitcoin/depends'
Extracting native_libmultiprocess...
(SHA256) /home/hebasto/dev/bitcoin/depends/sources/src-ipc-libmultiprocess.tar: OK
Preprocessing native_libmultiprocess...
Configuring native_libmultiprocess...
-- The CXX compiler identificat
...
💬 hebasto commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3201686643)
`native_libmultiprocess` [fails](https://github.com/bitcoin/bitcoin/pull/32345) to build on OpenBSD.
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3201686643)
`native_libmultiprocess` [fails](https://github.com/bitcoin/bitcoin/pull/32345) to build on OpenBSD.
💬 hebasto commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201688128)
cc @ryanofsky
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201688128)
cc @ryanofsky
💬 ismaelsadeeq commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2285961327)
The `test_sub_1s_per_vb_estimates` is not even necessary hence can be removed because we have a general test in `sanity_check_estimates_range`.
I added `test_sub_1s_per_vb_estimates` to convince reviewers and myself that it works.
However that said, I don't think its okay to bundle test together like this, but curious to know your opinion on why we would want to avoid adding the test.
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2285961327)
The `test_sub_1s_per_vb_estimates` is not even necessary hence can be removed because we have a general test in `sanity_check_estimates_range`.
I added `test_sub_1s_per_vb_estimates` to convince reviewers and myself that it works.
However that said, I don't think its okay to bundle test together like this, but curious to know your opinion on why we would want to avoid adding the test.
💬 Sjors commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201700143)
@vasild if I remember correctly you had a PR to add ?BSD CI coverage?
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201700143)
@vasild if I remember correctly you had a PR to add ?BSD CI coverage?
💬 polespinasa commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-3201704367)
ACK ba33047b6f3ee468eab250c8515e92b595e08322 reviewed the code and build and run the tests locally.
Adding a mempool-based fee estimation is a great step towards improving Core fee estimation.
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-3201704367)
ACK ba33047b6f3ee468eab250c8515e92b595e08322 reviewed the code and build and run the tests locally.
Adding a mempool-based fee estimation is a great step towards improving Core fee estimation.
🤔 enirox001 reviewed a pull request: "wallet: Remove isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523#pullrequestreview-3133328366)
ACK 620abe9 — built & tested locally; wallet tests passed. Behavior-preserving refactor; removing isminetype in favor of explicit booleans improves clarity.
Non-blocking nit: wallet_migration.py still references ISMINE_SPENDABLE / ISMINE_NO in comments. Functionally the tests use the boolean ismine; could you briefly explain why you kept the enum names in the comments, or consider changing them to ismine == True / ismine == False for consistency?
(https://github.com/bitcoin/bitcoin/pull/32523#pullrequestreview-3133328366)
ACK 620abe9 — built & tested locally; wallet tests passed. Behavior-preserving refactor; removing isminetype in favor of explicit booleans improves clarity.
Non-blocking nit: wallet_migration.py still references ISMINE_SPENDABLE / ISMINE_NO in comments. Functionally the tests use the boolean ismine; could you briefly explain why you kept the enum names in the comments, or consider changing them to ismine == True / ismine == False for consistency?
💬 hebasto commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201706852)
> [@vasild](https://github.com/vasild) if I remember correctly you had a PR to add ?BSD CI coverage?
For Bitcoin Core, see https://github.com/hebasto/bitcoin-core-nightly.
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201706852)
> [@vasild](https://github.com/vasild) if I remember correctly you had a PR to add ?BSD CI coverage?
For Bitcoin Core, see https://github.com/hebasto/bitcoin-core-nightly.
💬 polespinasa commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2285977318)
Less code the better :)
It's not a new or modified feature to be tested, it's just a value that has been lowered.
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2285977318)
Less code the better :)
It's not a new or modified feature to be tested, it's just a value that has been lowered.
💬 achow101 commented on pull request "wallet: Remove isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3201717327)
> Non-blocking nit: wallet_migration.py still references ISMINE_SPENDABLE / ISMINE_NO in comments. Functionally the tests use the boolean ismine; could you briefly explain why you kept the enum names in the comments, or consider changing them to ismine == True / ismine == False for consistency?
https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3190062828
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3201717327)
> Non-blocking nit: wallet_migration.py still references ISMINE_SPENDABLE / ISMINE_NO in comments. Functionally the tests use the boolean ismine; could you briefly explain why you kept the enum names in the comments, or consider changing them to ismine == True / ismine == False for consistency?
https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3190062828
💬 Sjors commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201724485)
Maybe we could have a label like "moar-ci" that runs these extra systems? Similar to how we can request guix builds.
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3201724485)
Maybe we could have a label like "moar-ci" that runs these extra systems? Similar to how we can request guix builds.
💬 TheBlueMatt commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201748467)
> The only difference between the binary that supports the -ipcbind option and the binary that does not support it
Forgive my ignorance, but ISTM a simple solution to @sipa's concern, then, would be for the *only* release `bitcoind` binary to be the `bitcoin-node` one. Is there some reason why someone should prefer to have run a binary which is simply missing the option (given release builds optionally provide it)?
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201748467)
> The only difference between the binary that supports the -ipcbind option and the binary that does not support it
Forgive my ignorance, but ISTM a simple solution to @sipa's concern, then, would be for the *only* release `bitcoind` binary to be the `bitcoin-node` one. Is there some reason why someone should prefer to have run a binary which is simply missing the option (given release builds optionally provide it)?
🤔 janb84 reviewed a pull request: "test: modify logging_filesize_rate_limit params"
(https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3133387403)
re ACK 5dda364c4b1965da586db7b81de8be90b6919414
changes since last ACK:
- more informative test failure loggin
(https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3133387403)
re ACK 5dda364c4b1965da586db7b81de8be90b6919414
changes since last ACK:
- more informative test failure loggin
💬 fanquake commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3201822267)
> Maybe add a note so we don't forget?
Any preference to where we add that?
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3201822267)
> Maybe add a note so we don't forget?
Any preference to where we add that?
📝 polespinasa opened a pull request: "doc: truc packages allow sub min feerate transactions"
(https://github.com/bitcoin/bitcoin/pull/33220)
Fixes https://github.com/bitcoin/bitcoin/issues/32067
Some policy documentation is outdated since TRUC. This PR aims to update the documentation to the actual policy state.
(https://github.com/bitcoin/bitcoin/pull/33220)
Fixes https://github.com/bitcoin/bitcoin/issues/32067
Some policy documentation is outdated since TRUC. This PR aims to update the documentation to the actual policy state.
💬 polespinasa commented on issue "doc: Mempool Policy documentation Outdated since TRUC":
(https://github.com/bitcoin/bitcoin/issues/32067#issuecomment-3201836465)
Went ahead and opened a PR https://github.com/bitcoin/bitcoin/pull/33220 as draft to not create much noise.
@glozow feel free to review and suggest text :)
(https://github.com/bitcoin/bitcoin/issues/32067#issuecomment-3201836465)
Went ahead and opened a PR https://github.com/bitcoin/bitcoin/pull/33220 as draft to not create much noise.
@glozow feel free to review and suggest text :)