💬 joostjager commented on pull request "policy: make unstructured annex standard":
(https://github.com/bitcoin/bitcoin/pull/27926#issuecomment-1606255813)
@luke-jr annex covenants, see link in PR description
(https://github.com/bitcoin/bitcoin/pull/27926#issuecomment-1606255813)
@luke-jr annex covenants, see link in PR description
📝 ismaelsadeeq opened a pull request: "bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE when user specifies fee_rate "
(https://github.com/bitcoin/bitcoin/pull/27969)
Fixes #26973
When using the `bumpfee` RPC and manually specifying `fee_rate`, there should be no requirement that the new fee must be at least the sum of the original fee and `incrementalFee` (maximum of `relayIncrementalFee` and `WALLET_INCREMENTAL_RELAY_FEE`).
This restriction should only apply when user did not specify `fee_rate`.
> because the GUI doesn't let the user specify the new fee rate yet (https://github.com/bitcoin-core/gui/issues/647), it would be very annoying to have to bu
...
(https://github.com/bitcoin/bitcoin/pull/27969)
Fixes #26973
When using the `bumpfee` RPC and manually specifying `fee_rate`, there should be no requirement that the new fee must be at least the sum of the original fee and `incrementalFee` (maximum of `relayIncrementalFee` and `WALLET_INCREMENTAL_RELAY_FEE`).
This restriction should only apply when user did not specify `fee_rate`.
> because the GUI doesn't let the user specify the new fee rate yet (https://github.com/bitcoin-core/gui/issues/647), it would be very annoying to have to bu
...
🤔 ariard reviewed a pull request: "policy: make unstructured annex standard"
(https://github.com/bitcoin/bitcoin/pull/27926#pullrequestreview-1497263042)
IIRC from #19645 and beyond, witnesses could be concatenate on the flight by transaction-relaying node, therefore ensuring the best feerate transaction propagates. As it’s opening it’s own wormhole of DoS issues, better to pursue the conversation on the mailing list.
(https://github.com/bitcoin/bitcoin/pull/27926#pullrequestreview-1497263042)
IIRC from #19645 and beyond, witnesses could be concatenate on the flight by transaction-relaying node, therefore ensuring the best feerate transaction propagates. As it’s opening it’s own wormhole of DoS issues, better to pursue the conversation on the mailing list.
💬 ariard commented on pull request "policy: make unstructured annex standard":
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241353751)
I think it makes sense to have a `annexrelay` option in `src/init.cpp` with `DEFAULT_ANNEXRELAY_ENABLE=false`, therefore to have node operators running nodes on performance-constrained hosts to opt-out from DoS concerns arising from the processing of the annex itself.
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241353751)
I think it makes sense to have a `annexrelay` option in `src/init.cpp` with `DEFAULT_ANNEXRELAY_ENABLE=false`, therefore to have node operators running nodes on performance-constrained hosts to opt-out from DoS concerns arising from the processing of the annex itself.
💬 ariard commented on pull request "policy: make unstructured annex standard":
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241352665)
I think we can constify the value, e.g `PER_INPUT_MAX_ANNEX_SIZE`, generally this is unclear if we would like a per-input annex limit and a max transaction all annexes limits, e.g `MAX_ANNEX_BUDGET`.
Splitting the limit enables flow where inputs can be combined in a non-deterministic order, e.g BIP174’s [combiner](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#combiner), and reject in function of “witness weight slots” that can be distributed by such combiner, which makes sens
...
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241352665)
I think we can constify the value, e.g `PER_INPUT_MAX_ANNEX_SIZE`, generally this is unclear if we would like a per-input annex limit and a max transaction all annexes limits, e.g `MAX_ANNEX_BUDGET`.
Splitting the limit enables flow where inputs can be combined in a non-deterministic order, e.g BIP174’s [combiner](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#combiner), and reject in function of “witness weight slots” that can be distributed by such combiner, which makes sens
...
💬 ariard commented on pull request "policy: make unstructured annex standard":
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241355849)
Should we introduce a limit on the number of inputs supported in a transaction opted-in annex ? To make it easier to reason on worst-case scenario from a second-layer perspective in case of multi-party flow e.g dual-funding. This number of inputs could be tied to the transaction’s `nVersion` field.
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241355849)
Should we introduce a limit on the number of inputs supported in a transaction opted-in annex ? To make it easier to reason on worst-case scenario from a second-layer perspective in case of multi-party flow e.g dual-funding. This number of inputs could be tied to the transaction’s `nVersion` field.
💬 ariard commented on pull request "policy: make unstructured annex standard":
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241344893)
I think here we might prefer to _not_ discount the 0x00 signaling byte from the 265 byte anti-annex-inflation-attacks limit, otherwise it sounds to me we’re introducing a coupling effect between the max witness size one can sign for and the tag signaling byte format itself. Such tag signaling byte format might acquire consensus semantic in the future, and I think we would prefer to reserve the evolvability in function of consensus changes here.
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241344893)
I think here we might prefer to _not_ discount the 0x00 signaling byte from the 265 byte anti-annex-inflation-attacks limit, otherwise it sounds to me we’re introducing a coupling effect between the max witness size one can sign for and the tag signaling byte format itself. Such tag signaling byte format might acquire consensus semantic in the future, and I think we would prefer to reserve the evolvability in function of consensus changes here.
💬 ariard commented on pull request "policy: make unstructured annex standard":
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241358413)
If we can move the annex policy-only code in its own `src/policy/annex` like we have at the image of `src/policy/packages.h` for code modularity. I think in the discussion of [#1381](https://github.com/bitcoin/bips/pull/1381) there has been question to reuse the annex in the future for [Grafroot](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-February/015700.html).
(https://github.com/bitcoin/bitcoin/pull/27926#discussion_r1241358413)
If we can move the annex policy-only code in its own `src/policy/annex` like we have at the image of `src/policy/packages.h` for code modularity. I think in the discussion of [#1381](https://github.com/bitcoin/bips/pull/1381) there has been question to reuse the annex in the future for [Grafroot](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-February/015700.html).
💬 mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241361274)
nit: unrelated whitespace change
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241361274)
nit: unrelated whitespace change
💬 mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241362625)
I think `addrConnect` can't be used here, because it can be empty if `pszDest` is set (that's why the CI fails too). You could use `pnode->addr` instead, which is set from either `addrConnect` or `pszDest` in `ConnectNode()`!
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241362625)
I think `addrConnect` can't be used here, because it can be empty if `pszDest` is set (that's why the CI fails too). You could use `pnode->addr` instead, which is set from either `addrConnect` or `pszDest` in `ConnectNode()`!
💬 mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241361033)
here and in `test/util/net.h` - this is still using `ConnectedThroughNetwork` (see previous comment).
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241361033)
here and in `test/util/net.h` - this is still using `ConnectedThroughNetwork` (see previous comment).
📝 Jezebel5134567 opened a pull request: "Create bitcoinhh"
(https://github.com/bitcoin/bitcoin/pull/27970)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/27970)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 Jezebel5134567 commented on pull request "Create bitcoinhh":
(https://github.com/bitcoin/bitcoin/pull/27970#issuecomment-1606398607)
btc
(https://github.com/bitcoin/bitcoin/pull/27970#issuecomment-1606398607)
btc
💬 ariard commented on pull request "p2p: Advertise `NODE_FULL_RBF` and connect to 4 outbound full-rbf peers if `-mempoolfullrbf` is set":
(https://github.com/bitcoin/bitcoin/pull/25600#issuecomment-1606414648)
In the context of #27926, I think we might have to reconsider automatic transaction-relay preferential peering on the table, therefore allowing full-node operators to opt-in with `annexrelay=true`.
Beyond, such use-case, I think the preferential peering could be abstracted like we have for consensus-deployment code in `/consensus/params.h` and `DeploymentHeight()` in src/validation.cpp`. This could be re-used not only for `mempoolfullrbf` and future replacemnt logic upgrade, but also for `nVe
...
(https://github.com/bitcoin/bitcoin/pull/25600#issuecomment-1606414648)
In the context of #27926, I think we might have to reconsider automatic transaction-relay preferential peering on the table, therefore allowing full-node operators to opt-in with `annexrelay=true`.
Beyond, such use-case, I think the preferential peering could be abstracted like we have for consensus-deployment code in `/consensus/params.h` and `DeploymentHeight()` in src/validation.cpp`. This could be re-used not only for `mempoolfullrbf` and future replacemnt logic upgrade, but also for `nVe
...
⚠️ tansanDOTeth opened an issue: "Keep getting errors after a while of syncing"
(https://github.com/bitcoin/bitcoin/issues/27972)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Spent the last few weeks trying to sync a full node and keep getting a wild error after making lots of progress. I had to restart at least 3 times, but decided to finally report it here.
> 2023-06-26T04:05:04Z LevelDB read failure: Corruption: block checksum mismatch: /Volumes/Crucial X8/BTC Blockchain Data/chainstate/031566.ldb
2023-06-26T04:05:04Z Fatal LevelDB error: Corruption: blo
...
(https://github.com/bitcoin/bitcoin/issues/27972)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Spent the last few weeks trying to sync a full node and keep getting a wild error after making lots of progress. I had to restart at least 3 times, but decided to finally report it here.
> 2023-06-26T04:05:04Z LevelDB read failure: Corruption: block checksum mismatch: /Volumes/Crucial X8/BTC Blockchain Data/chainstate/031566.ldb
2023-06-26T04:05:04Z Fatal LevelDB error: Corruption: blo
...
✅ achow101 closed a pull request: "Create bitcoinhh"
(https://github.com/bitcoin/bitcoin/pull/27970)
(https://github.com/bitcoin/bitcoin/pull/27970)
📝 achow101 locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/27970)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/27970)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 tansanDOTeth commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/21724#issuecomment-1606570109)
> I had this checksum LevelDB problem frequently in the last weeks, following this answer [#10897 (comment)](https://github.com/bitcoin/bitcoin/issues/10897#issuecomment-317355977) I managed to solve it, the HD I have been using to keep a copy of bitcoins blockchain is not working properly anymore, copying everything to a new HD and using it completely solve the errors.
I read that comment, but not sure what I have to do run tests against my external. Do you have more instructions written som
...
(https://github.com/bitcoin/bitcoin/issues/21724#issuecomment-1606570109)
> I had this checksum LevelDB problem frequently in the last weeks, following this answer [#10897 (comment)](https://github.com/bitcoin/bitcoin/issues/10897#issuecomment-317355977) I managed to solve it, the HD I have been using to keep a copy of bitcoins blockchain is not working properly anymore, copying everything to a new HD and using it completely solve the errors.
I read that comment, but not sure what I have to do run tests against my external. Do you have more instructions written som
...
💬 MarcoFalke commented on pull request "bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE when user specifies fee_rate":
(https://github.com/bitcoin/bitcoin/pull/27969#discussion_r1241602260)
missing test for larger than original fee rate, but not enough to be larger by incrementalrelayfee?
(https://github.com/bitcoin/bitcoin/pull/27969#discussion_r1241602260)
missing test for larger than original fee rate, but not enough to be larger by incrementalrelayfee?
💬 S3RK commented on pull request "Disable and uncheck blank when private keys are disabled":
(https://github.com/bitcoin-core/gui/pull/739#issuecomment-1606849299)
I'm nor a GUI code expert neither a UX (user experience) expert, so take all of this with a healthy skepticism.
The code changes look good to me, but I'm unsure about whether this is a better UX.
Code review ACK 9ea31eba04ff8dcb9d7d993ce28bb10731a35177
I'm unsure if it's a clear win for UX because there's no hint to the user why the checkboxes change their state. I also like the idea with radio buttons and I'd be happy to review a new PR or re-review this one though.
Maybe change the de
...
(https://github.com/bitcoin-core/gui/pull/739#issuecomment-1606849299)
I'm nor a GUI code expert neither a UX (user experience) expert, so take all of this with a healthy skepticism.
The code changes look good to me, but I'm unsure about whether this is a better UX.
Code review ACK 9ea31eba04ff8dcb9d7d993ce28bb10731a35177
I'm unsure if it's a clear win for UX because there's no hint to the user why the checkboxes change their state. I also like the idea with radio buttons and I'd be happy to review a new PR or re-review this one though.
Maybe change the de
...