💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1881049934)
> > I think a little on switching to string-based settings, i.e. `-rpccookieperms=group`, but this quickly gets more ugly inside, as IMO the usual use-case is to be setting multiple values in the same permission. i.e. setting just `user` (`040`) means that the owner-user will NOT be able to read the cookie, as user is checked against owner before group, and if a match is found, with no read perms, the checks are exited. I feel that users are likely to want to set `group` in _addition_ to `user`,
...

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1444673561)
the more work solutions would be to refactor everything to have a "subpackage eval" state, but juice is probably not worth the squeeze until cluster mempool, where "chunk eval" is likely the natural boundary

💬 sipa commented on pull request "Weaken serfloat tests":
(https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1881052463)
@fanquake What if you drop the last commit here?

💬 kristapsk commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1881053840)
> I don't think enabling write perms on a cookie (ever?!) that bitcoind has created makes sense?

Ahh, yes, `400`, `440`, `444`.

💬 hebasto commented on issue "qa: No access to `debug.log` in functional tests on Windows":
(https://github.com/bitcoin/bitcoin/issues/28529#issuecomment-1881119075)
https://github.com/bitcoin/bitcoin/actions/runs/7433919780/job/20227517742:
```
2024-01-06T21:17:20.926000Z TestFramework (INFO): Tests successful



stderr:
Traceback (most recent call last):

File "D:\a\bitcoin\bitcoin/test/functional/tool_utxo_to_sqlite.py", line 111, in <module>

UtxoToSqliteTest().main()

File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_framework.py", line 154, in main

exit_code = self.shutdown()

^^^^^^^^^^^^^^^

...

💬 apoelstra commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1881121970)
> Would something along the lines of https://github.com/ajtowns/bitcoin/commits/202309-evalscript/ be an interesting alternative?

For my part I would rather have a library function than a CLI tool.

💬 recursive-rat4 commented on pull request "net: create I2P sessions using both ECIES-X25519 and ElGamal encryption":
(https://github.com/bitcoin/bitcoin/pull/29200#issuecomment-1881124957)
ACK 9d728916b27e18efc6f8839770ed5ec14789fc08

I2P 2.3.0 shows both encryption keys.

🤔 furszy reviewed a pull request: "sqlite: Disallow writing from multiple `SQLiteBatch`s"
(https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1809248451)
Another possible deadlock can arise when the txn abortion fails during the destruction of the batch object.
Essentially, if the txn abortion fails and the object is destructed, the semaphore remains locked indefinitely, leading to a deadlock in any subsequent write operation.

Made a test replicating this behavior; cherry-pick 5d2b79a65dcb10abdd349471c125595d677909ab and fbd59f89d3f5ed7bc026efa47d1f78c5cee04291 (from https://github.com/furszy/bitcoin-core/commits/sqlite-concurrent-writes/ )

💬 fanquake commented on pull request "Weaken serfloat tests":
(https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1881144857)
> What if you drop the last commit here?

Tests are passing: https://github.com/Homebrew/homebrew-core/actions/runs/7448770957/job/20264045705?pr=156745.

🚀 fanquake merged a pull request: "wallet: Migrate entire address book entries to watchonly and solvables too"
(https://github.com/bitcoin/bitcoin/pull/28610)

📝 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.

📝 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.

💬 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

💬 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 :)

👍 hebasto approved a pull request: "RFC: Deprecate libconsensus"
(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.

💬 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

💬 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
...

💬 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

💬 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.