📝 hebasto opened a pull request: "build: Drop `ALLOW_HOST_PACKAGES` support in depends"
(https://github.com/bitcoin/bitcoin/pull/29203)
The `ALLOW_HOST_PACKAGES` variable was introduced in bitcoin#10508 "to speed up build and avoid timeout".
It is no longer the case for our CI infrastructure, which uses self- hosted persistent workers and depends caching.
In the current circumstances, it does not seem worth porting this feature to the upcoming CMake-based build system.
(https://github.com/bitcoin/bitcoin/pull/29203)
The `ALLOW_HOST_PACKAGES` variable was introduced in bitcoin#10508 "to speed up build and avoid timeout".
It is no longer the case for our CI infrastructure, which uses self- hosted persistent workers and depends caching.
In the current circumstances, it does not seem worth porting this feature to the upcoming CMake-based build system.
📝 furszy opened a pull request: "test: wallet migration, add coverage for tx extra data"
(https://github.com/bitcoin/bitcoin/pull/29204)
Quick follow-up to #28610, coming from https://github.com/bitcoin/bitcoin/pull/28610#pullrequestreview-1802823938.
Verifying that the 'replaced_by_txid' and 'replaces_txid' tx data is preserved after migration,
as well as the extra tx comments.
(https://github.com/bitcoin/bitcoin/pull/29204)
Quick follow-up to #28610, coming from https://github.com/bitcoin/bitcoin/pull/28610#pullrequestreview-1802823938.
Verifying that the 'replaced_by_txid' and 'replaces_txid' tx data is preserved after migration,
as well as the extra tx comments.
💬 hebasto commented on issue "CMake-based build system tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-1881191524)
Heads up: here are suggestions regarding the current features that are _not_ expected being ported to the CMake-based build system:
- https://github.com/bitcoin/bitcoin/pull/29185
- https://github.com/bitcoin/bitcoin/pull/29189
- https://github.com/bitcoin/bitcoin/pull/29203
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-1881191524)
Heads up: here are suggestions regarding the current features that are _not_ expected being ported to the CMake-based build system:
- https://github.com/bitcoin/bitcoin/pull/29185
- https://github.com/bitcoin/bitcoin/pull/29189
- https://github.com/bitcoin/bitcoin/pull/29203
💬 instagibbs commented on pull request "test: Add test case for spending bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#discussion_r1444815132)
> If the tx-to-be-spent is already confirmed, there shouldn't be any problem having a spend of a 20-of-20 checkmultisig in the mempool
we have tests for this? If not, this is probably the right thing to cover I guewss :)
(https://github.com/bitcoin/bitcoin/pull/29120#discussion_r1444815132)
> If the tx-to-be-spent is already confirmed, there shouldn't be any problem having a spend of a 20-of-20 checkmultisig in the mempool
we have tests for this? If not, this is probably the right thing to cover I guewss :)
👍 hebasto approved a pull request: "RFC: Deprecate libconsensus"
(https://github.com/bitcoin/bitcoin/pull/29189#pullrequestreview-1809338454)
ACK a6745f8be306a0aeeb7fb070297ba503321891d0, the diff is correct.
(https://github.com/bitcoin/bitcoin/pull/29189#pullrequestreview-1809338454)
ACK a6745f8be306a0aeeb7fb070297ba503321891d0, the diff is correct.
💬 instagibbs commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444825855)
`testmempoolaccept` is basically shoving all transactions, whether or not they're a pacakge, into `AcceptMultipleTransactions`.
I'm not sure it makes sense due to this.
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444825855)
`testmempoolaccept` is basically shoving all transactions, whether or not they're a pacakge, into `AcceptMultipleTransactions`.
I'm not sure it makes sense due to this.
💬 kevkevinpal commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#issuecomment-1881230813)
> Still working on this? I'll be available to re-review quickly after comments are addressed.
yes sorry I can try and make some followup changes to the comments soon
(https://github.com/bitcoin/bitcoin/pull/28885#issuecomment-1881230813)
> Still working on this? I'll be available to re-review quickly after comments are addressed.
yes sorry I can try and make some followup changes to the comments soon
💬 instagibbs commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444836670)
Ignoring topo restrictions in `AcceptPackage` since `C` should have `A` as a direct ancestor,
> The child transactions might be greater than the maxfeerate, but as a package might not be. In this case, we might reject the child transactions and accept the subpackage, which might not be incentive compatible at the current fee estimates.
This is prior to any relay, of course restricting feerates is incentive compatible once published on the open network!
> If maxfeerate is 50s/vb we will
...
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444836670)
Ignoring topo restrictions in `AcceptPackage` since `C` should have `A` as a direct ancestor,
> The child transactions might be greater than the maxfeerate, but as a package might not be. In this case, we might reject the child transactions and accept the subpackage, which might not be incentive compatible at the current fee estimates.
This is prior to any relay, of course restricting feerates is incentive compatible once published on the open network!
> If maxfeerate is 50s/vb we will
...
💬 instagibbs commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444841601)
startup config? Maybe? Could open an issue
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444841601)
startup config? Maybe? Could open an issue
💬 sipa commented on pull request "Weaken serfloat tests":
(https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1881252737)
Dropped the last commit, and improved the code changes a bit. For backport, if needed, the first commit suffices.
(https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1881252737)
Dropped the last commit, and improved the code changes a bit. For backport, if needed, the first commit suffices.
✅ achow101 closed a pull request: "Open fully encrypted wallets"
(https://github.com/bitcoin-core/gui/pull/747)
(https://github.com/bitcoin-core/gui/pull/747)
💬 maflcko commented on pull request "build: Drop `ALLOW_HOST_PACKAGES` support in depends":
(https://github.com/bitcoin/bitcoin/pull/29203#discussion_r1444886977)
```suggestion
export PACKAGES="gcc-10 g++-10 python3-zmq"
```
(https://github.com/bitcoin/bitcoin/pull/29203#discussion_r1444886977)
```suggestion
export PACKAGES="gcc-10 g++-10 python3-zmq"
```
💬 hebasto commented on pull request "build: Drop `ALLOW_HOST_PACKAGES` support in depends":
(https://github.com/bitcoin/bitcoin/pull/29203#discussion_r1444892896)
Thanks! Fixed.
(https://github.com/bitcoin/bitcoin/pull/29203#discussion_r1444892896)
Thanks! Fixed.
👋 achow101's pull request is ready for review: "Dialog for allowing the user to choose the change output when bumping a tx"
(https://github.com/bitcoin-core/gui/pull/700)
(https://github.com/bitcoin-core/gui/pull/700)
💬 instagibbs commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444942956)
added a mention of the default being 0
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444942956)
added a mention of the default being 0
💬 instagibbs commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444943017)
taken
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444943017)
taken
💬 jamesob commented on pull request "test: wallet migration, add coverage for tx extra data":
(https://github.com/bitcoin/bitcoin/pull/29204#issuecomment-1881425364)
Nice, ACK https://github.com/bitcoin/bitcoin/pull/29204/commits/016cc807f77a9128d430a0df1edd133521628a33
(https://github.com/bitcoin/bitcoin/pull/29204#issuecomment-1881425364)
Nice, ACK https://github.com/bitcoin/bitcoin/pull/29204/commits/016cc807f77a9128d430a0df1edd133521628a33
💬 jamesob commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1881435270)
Two ACKs on a test change - RFM?
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1881435270)
Two ACKs on a test change - RFM?
📝 fanquake opened a pull request: "build: always set `-g -O2` in `CORE_CXXFLAGS`"
(https://github.com/bitcoin/bitcoin/pull/29205)
Rather than trying to sporadically rely on / override Autoconf default behaviour. Just always override, and always set the flags we want (which are the same as the Autoconf defaults).
Removes the need for duplicate code to clear (if not overridden) `CXXFLAGS`.
Fixes cases of "missing" `-O2`. i.e this PR when running a Valgrind CI job with changes here:
```bash
CXXFLAGS = -g -O2 -fdebug-prefix-map=$(abs_top_srcdir)=. -Wstack-protector -fstack-protector-all -mbranch-protection=bt
...
(https://github.com/bitcoin/bitcoin/pull/29205)
Rather than trying to sporadically rely on / override Autoconf default behaviour. Just always override, and always set the flags we want (which are the same as the Autoconf defaults).
Removes the need for duplicate code to clear (if not overridden) `CXXFLAGS`.
Fixes cases of "missing" `-O2`. i.e this PR when running a Valgrind CI job with changes here:
```bash
CXXFLAGS = -g -O2 -fdebug-prefix-map=$(abs_top_srcdir)=. -Wstack-protector -fstack-protector-all -mbranch-protection=bt
...
💬 jamesob commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1443338475)
This doesn't merit any updating in non-test code because we never call `AddCoin()` within a `try` outside of tests, right?
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1443338475)
This doesn't merit any updating in non-test code because we never call `AddCoin()` within a `try` outside of tests, right?