💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3201535092)
Addressed a number of comments, and rebased on top of #31802.
I will however not have (much) time to push this further forward until the end of this month. @Sjors or @ryanofsky, or someone else, feel free to take this over and/or integrate into other PRs.
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3201535092)
Addressed a number of comments, and rebased on top of #31802.
I will however not have (much) time to push this further forward until the end of this month. @Sjors or @ryanofsky, or someone else, feel free to take this over and/or integrate into other PRs.
📝 ismaelsadeeq opened a pull request: "mini miner: enable `Linearize` return package feerates"
(https://github.com/bitcoin/bitcoin/pull/33216)
This PR adds supports for mini miner to return the package fee rates of the linearized block templates #30079
Motivated by https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3191510709
(https://github.com/bitcoin/bitcoin/pull/33216)
This PR adds supports for mini miner to return the package fee rates of the linearized block templates #30079
Motivated by https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3191510709
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285853200)
Changed to print to stdout, seems to work.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285853200)
Changed to print to stdout, seems to work.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285853964)
Fixed, see elsewhere.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285853964)
Fixed, see elsewhere.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285854554)
Added the ignore annotation.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285854554)
Added the ignore annotation.
💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3201559444)
> Feel free to ignore me if this PR is close to merge, but it does feel like there are two PRs here. The first 3 commits seem more about updates to miniminer, and then the 4th commit does what the title of this PR indicates.
>
> It may help to attract more review by breaking out the first 3 commits into their own PR, using this PR as a motivating use case. Either way, I plan to dig in more next week and review, but wanted to mention this.
Not close to merge I think, so far this PR received
...
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3201559444)
> Feel free to ignore me if this PR is close to merge, but it does feel like there are two PRs here. The first 3 commits seem more about updates to miniminer, and then the 4th commit does what the title of this PR indicates.
>
> It may help to attract more review by breaking out the first 3 commits into their own PR, using this PR as a motivating use case. Either way, I plan to dig in more next week and review, but wanted to mention this.
Not close to merge I think, so far this PR received
...
📝 ismaelsadeeq converted_to_draft a pull request: "Fee Estimation: Ignore all transactions that are CPFP'd"
(https://github.com/bitcoin/bitcoin/pull/30079)
Another attempt at #25380 with an alternate approach
This PR updates `CBlockPolicyEstimator` to ignore all transactions that are CPFP'd by some child when a new block is received.
This fixes the assumption that all transactions are confirmed solely because of their fee rate.
As some transactions are confirmed due to a CPFP by some child.
This approach linearize all transactions removed from the mempool because of the new block, and only ignore transactions whose mining score is not
...
(https://github.com/bitcoin/bitcoin/pull/30079)
Another attempt at #25380 with an alternate approach
This PR updates `CBlockPolicyEstimator` to ignore all transactions that are CPFP'd by some child when a new block is received.
This fixes the assumption that all transactions are confirmed solely because of their fee rate.
As some transactions are confirmed due to a CPFP by some child.
This approach linearize all transactions removed from the mempool because of the new block, and only ignore transactions whose mining score is not
...
💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3201582620)
Putting in draft because of a dependent PR.
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3201582620)
Putting in draft because of a dependent PR.
📝 fanquake opened a pull request: "depends: remove xinerama extension from libxcb"
(https://github.com/bitcoin/bitcoin/pull/33217)
This is listed on https://doc.qt.io/qt-5.15/linux-requirements.html as "recommended", and doesn't seem to be needed (only used for windowing over multiple screens support?) , and the fact that it's no-longer installed by default on modern linux distros (i.e Ubuntu), is annoying/confusing for users. See:
https://github.com/bitcoin/bitcoin/issues/30061
https://github.com/bitcoin/bitcoin/issues/32097
https://github.com/bitcoin/bitcoin/pull/33197
https://bitcoin.stackexchange.com/questions/122
...
(https://github.com/bitcoin/bitcoin/pull/33217)
This is listed on https://doc.qt.io/qt-5.15/linux-requirements.html as "recommended", and doesn't seem to be needed (only used for windowing over multiple screens support?) , and the fact that it's no-longer installed by default on modern linux distros (i.e Ubuntu), is annoying/confusing for users. See:
https://github.com/bitcoin/bitcoin/issues/30061
https://github.com/bitcoin/bitcoin/issues/32097
https://github.com/bitcoin/bitcoin/pull/33197
https://bitcoin.stackexchange.com/questions/122
...
💬 stickies-v commented on pull request "test: modify logging_filesize_rate_limit params":
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3201612121)
re-ACK 5dda364c4b1965da586db7b81de8be90b6919414 for more helpful failure logging, no other changes
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3201612121)
re-ACK 5dda364c4b1965da586db7b81de8be90b6919414 for more helpful failure logging, no other changes
🤔 polespinasa reviewed a pull request: "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates"
(https://github.com/bitcoin/bitcoin/pull/33199#pullrequestreview-3133246727)
cACK 413c81b134a16403cc0f64e21c3c0967c211b318
This change makes much sense to me, if the min relay fee is lowered the feerate tracking should also take that into account.
(https://github.com/bitcoin/bitcoin/pull/33199#pullrequestreview-3133246727)
cACK 413c81b134a16403cc0f64e21c3c0967c211b318
This change makes much sense to me, if the min relay fee is lowered the feerate tracking should also take that into account.
💬 polespinasa commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2285915354)
I was wondering if the new test could be avoided by defining `low_feerate` as MINIMUM_RELAY_FEE_RATE in the previous test `test_estimation_modes`.
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2285915354)
I was wondering if the new test could be avoided by defining `low_feerate` as MINIMUM_RELAY_FEE_RATE in the previous test `test_estimation_modes`.
📝 ismaelsadeeq opened a pull request: "fees: refactor: Create `policy/fees` dir and rename `fees.{h, cpp}` to `fees/block_policy_estimator{h, cpp}`"
(https://github.com/bitcoin/bitcoin/pull/33218)
This PR is a simple refactoring that does four things:
1. Renames `test/policy_fee_tests.cpp` to `test/feerounder_tests.cpp`.
2. Renames `policy/fees.{h,cpp}` to `policy/fees/block_policy_estimator.{h,cpp}`.
3. Renames `policy/fees_args.cpp` to `policy/fees/block_policy_estimator_args.cpp`.
4. Modifies `estimateSmartFee` to return the block height at which the estimate was made by adding a `best_height` unsigned int value to the `FeeCalculation` struct.
**Motivation**
In preparation
...
(https://github.com/bitcoin/bitcoin/pull/33218)
This PR is a simple refactoring that does four things:
1. Renames `test/policy_fee_tests.cpp` to `test/feerounder_tests.cpp`.
2. Renames `policy/fees.{h,cpp}` to `policy/fees/block_policy_estimator.{h,cpp}`.
3. Renames `policy/fees_args.cpp` to `policy/fees/block_policy_estimator_args.cpp`.
4. Modifies `estimateSmartFee` to return the block height at which the estimate was made by adding a `best_height` unsigned int value to the `FeeCalculation` struct.
**Motivation**
In preparation
...
💬 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