:lock: fanquake locked an issue: "Any plans to implement the concepts Kaspa has?"
(https://github.com/bitcoin/bitcoin/issues/29105)
(https://github.com/bitcoin/bitcoin/issues/29105)
💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1859942803)
The code looks good. That's quite a small change and fully fixes the problem. Didn't repeat manual tests though, but there is an automatic test coverage. Great job!
CI failure seems relevant. I was able to reproduce locally, didn't have time to debug yet though. Will look tomorrow if nobody beats me to it.
```
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/
...
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1859942803)
The code looks good. That's quite a small change and fully fixes the problem. Didn't repeat manual tests though, but there is an automatic test coverage. Great job!
CI failure seems relevant. I was able to reproduce locally, didn't have time to debug yet though. Will look tomorrow if nobody beats me to it.
```
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/
...
💬 naumenkogs commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1429773492)
1393bf67b26c000483c750258ca9d1daeab8a2cb
not super clear why we won't do this `if full_outbound_delta > 0`?
Maybe a comment explaining why this could be possible in the first place could help (probably above the first if block).
Even now it's hard to tell if it's possible to be over the limit most-of-the-time, let alone if we add some new complexity for being over the limit more frequently (for some security reason or whatever).
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1429773492)
1393bf67b26c000483c750258ca9d1daeab8a2cb
not super clear why we won't do this `if full_outbound_delta > 0`?
Maybe a comment explaining why this could be possible in the first place could help (probably above the first if block).
Even now it's hard to tell if it's possible to be over the limit most-of-the-time, let alone if we add some new complexity for being over the limit more frequently (for some security reason or whatever).
💬 naumenkogs commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1429786276)
ad48667ec8e8d563550df768d0b45abf800662d9
gotta drop the second condition here?
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1429786276)
ad48667ec8e8d563550df768d0b45abf800662d9
gotta drop the second condition here?
💬 naumenkogs commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1429781523)
68769ff7c3252c7cfb8758459218979eacba7acb
You could expand this test by adding some irrelevant peers (e.g. ADDRFETCH), but no big deal.
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1429781523)
68769ff7c3252c7cfb8758459218979eacba7acb
You could expand this test by adding some irrelevant peers (e.g. ADDRFETCH), but no big deal.
💬 naumenkogs commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1429789777)
ad48667ec8e8d563550df768d0b45abf800662d9
might update this too `if (!Assume(state)) return` and drop the comment if you feel like it :)
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1429789777)
ad48667ec8e8d563550df768d0b45abf800662d9
might update this too `if (!Assume(state)) return` and drop the comment if you feel like it :)
💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-1859954716)
Not intended for merge. Not planning on rebasing right now since this should be built on top of cluster mempool and other WIP projects.
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-1859954716)
Not intended for merge. Not planning on rebasing right now since this should be built on top of cluster mempool and other WIP projects.
👍 Sjors approved a pull request: "wallet, mempool: propagete `checkChainLimits` error message to wallet"
(https://github.com/bitcoin/bitcoin/pull/28863#pullrequestreview-1786351038)
utACK 8dec9c560b53488c1e71d8f74241c7dce42cb387
(https://github.com/bitcoin/bitcoin/pull/28863#pullrequestreview-1786351038)
utACK 8dec9c560b53488c1e71d8f74241c7dce42cb387
💬 Sjors commented on pull request "wallet, mempool: propagete `checkChainLimits` error message to wallet":
(https://github.com/bitcoin/bitcoin/pull/28863#discussion_r1429805433)
```cpp
* @returns {} or the error reason if a limit is hit.
```
(https://github.com/bitcoin/bitcoin/pull/28863#discussion_r1429805433)
```cpp
* @returns {} or the error reason if a limit is hit.
```
💬 Sjors commented on pull request "wallet, mempool: propagete `checkChainLimits` error message to wallet":
(https://github.com/bitcoin/bitcoin/pull/28863#issuecomment-1859961874)
cc @glozow & @instagibbs
(https://github.com/bitcoin/bitcoin/pull/28863#issuecomment-1859961874)
cc @glozow & @instagibbs
💬 Sjors commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1859963041)
@aureleoules wen rebase? :-)
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1859963041)
@aureleoules wen rebase? :-)
👍 TheCharlatan approved a pull request: "wallet, mempool: propagete `checkChainLimits` error message to wallet"
(https://github.com/bitcoin/bitcoin/pull/28863#pullrequestreview-1786371835)
Re-ACK 8dec9c560b53488c1e71d8f74241c7dce42cb387
(https://github.com/bitcoin/bitcoin/pull/28863#pullrequestreview-1786371835)
Re-ACK 8dec9c560b53488c1e71d8f74241c7dce42cb387
💬 glozow commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1429824510)
2f11d8d07e9141c56f6f7cc4e406d73723ead6e3 instead of magic numbers, can you make this programmatic, i.e. the tx fee + delta?
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1429824510)
2f11d8d07e9141c56f6f7cc4e406d73723ead6e3 instead of magic numbers, can you make this programmatic, i.e. the tx fee + delta?
💬 glozow commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1429829307)
in 0d3a4abd805e3d4deec25f9902e762a7e693ce6b
This is just testing that you didn't submit the transaction. Did you mean to use the `getprioritisedtransactions` RPC instead?
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1429829307)
in 0d3a4abd805e3d4deec25f9902e762a7e693ce6b
This is just testing that you didn't submit the transaction. Did you mean to use the `getprioritisedtransactions` RPC instead?
💬 glozow commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1429816799)
question - why the indentation changes?
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1429816799)
question - why the indentation changes?
💬 naumenkogs commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1429854862)
Is it possible that they haven't sent us anything (the state is `KEY`), but disconnected us for other reason than not supporting `v2`? I assume it's possible if an alternative client implements whatever because this is not prohibited by any bips... But even in the current client, say that `IsBanned` is triggered?
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1429854862)
Is it possible that they haven't sent us anything (the state is `KEY`), but disconnected us for other reason than not supporting `v2`? I assume it's possible if an alternative client implements whatever because this is not prohibited by any bips... But even in the current client, say that `IsBanned` is triggered?
💬 aureleoules commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#discussion_r1429863730)
Fixed thanks
(https://github.com/bitcoin/bitcoin/pull/26045#discussion_r1429863730)
Fixed thanks
💬 aureleoules commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1860032906)
> @aureleoules wen rebase? :-)
🫡
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1860032906)
> @aureleoules wen rebase? :-)
🫡
💬 glozow commented on pull request "wallet, mempool: propagete `checkChainLimits` error message to wallet":
(https://github.com/bitcoin/bitcoin/pull/28863#issuecomment-1860046724)
utACK 8dec9c560b
(https://github.com/bitcoin/bitcoin/pull/28863#issuecomment-1860046724)
utACK 8dec9c560b
💬 glozow commented on pull request "refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition":
(https://github.com/bitcoin/bitcoin/pull/29086#issuecomment-1860058118)
Looking at the conflicts with kernel, v3, cluster mempool, etc., is this the kind of mempool refactor we should defer for now?
(https://github.com/bitcoin/bitcoin/pull/29086#issuecomment-1860058118)
Looking at the conflicts with kernel, v3, cluster mempool, etc., is this the kind of mempool refactor we should defer for now?