💬 epiccurious commented on issue "Update security.md with all PGP fingerprints":
(https://github.com/bitcoin/bitcoin/issues/29366#issuecomment-1922494863)
+1 to @1440000bytes, they can contact the listed people directly instead of `security@`.
(https://github.com/bitcoin/bitcoin/issues/29366#issuecomment-1922494863)
+1 to @1440000bytes, they can contact the listed people directly instead of `security@`.
🤔 mzumsande reviewed a pull request: "test: p2p: adhere to typical VERSION message protocol flow"
(https://github.com/bitcoin/bitcoin/pull/29353#pullrequestreview-1857950026)
ACK c340503b67585b631909584f1acaa327f5af25d3
I ran the functional test suite both with and without `--v2transport` on this branch and both runs succeeded.
(https://github.com/bitcoin/bitcoin/pull/29353#pullrequestreview-1857950026)
ACK c340503b67585b631909584f1acaa327f5af25d3
I ran the functional test suite both with and without `--v2transport` on this branch and both runs succeeded.
💬 epiccurious commented on pull request "test: p2p: adhere to typical VERSION message protocol flow":
(https://github.com/bitcoin/bitcoin/pull/29353#issuecomment-1922597240)
utACK c340503b67585b631909584f1acaa327f5af25d3
(https://github.com/bitcoin/bitcoin/pull/29353#issuecomment-1922597240)
utACK c340503b67585b631909584f1acaa327f5af25d3
💬 epiccurious commented on pull request "refactor: Remove excess reserve() call for SecureString":
(https://github.com/bitcoin/bitcoin/pull/29364#issuecomment-1922599794)
Tested ACK. Passed the functional test runner.
(https://github.com/bitcoin/bitcoin/pull/29364#issuecomment-1922599794)
Tested ACK. Passed the functional test runner.
💬 daniel-btc-nym commented on issue "getJsonToken assumes underlying string is null-terminated but requires end pointer":
(https://github.com/bitcoin/bitcoin/issues/28260#issuecomment-1922687448)
I'm working on it. Plan is to harden the function to not read at end and after. I have some initial small draft changes, and want to add testing. Maybe, we can even highlight the issue via additional fuzz+msan tests.
(https://github.com/bitcoin/bitcoin/issues/28260#issuecomment-1922687448)
I'm working on it. Plan is to harden the function to not read at end and after. I have some initial small draft changes, and want to add testing. Maybe, we can even highlight the issue via additional fuzz+msan tests.
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475452360)
This could be done in a follow-up if people want it, because currently the conflicting transactions are stored in the `CWalletTx`, and not the transaction state, so this function is not aware of the txids of the mempool conflicts.
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475452360)
This could be done in a follow-up if people want it, because currently the conflicting transactions are stored in the `CWalletTx`, and not the transaction state, so this function is not aware of the txids of the mempool conflicts.
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475453805)
Done
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475453805)
Done
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475453968)
Done
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475453968)
Done
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475454363)
Thanks, that should be fixed now
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475454363)
Thanks, that should be fixed now
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475454524)
Done
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475454524)
Done
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475454837)
Good point, the tests now check the exact contents.
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475454837)
Good point, the tests now check the exact contents.
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475455088)
I've added an assertion that the tx isn't in the mempool.
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475455088)
I've added an assertion that the tx isn't in the mempool.
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475457535)
Yes, I will add these in a follow-up PR.
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1475457535)
Yes, I will add these in a follow-up PR.
💬 DanaBCannon commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1475536100)
Im Open For Business
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1475536100)
Im Open For Business
💬 stratospher commented on pull request "test: p2p: adhere to typical VERSION message protocol flow":
(https://github.com/bitcoin/bitcoin/pull/29353#issuecomment-1922955178)
tested ACK c340503b67585b631909584f1acaa327f5af25d3. very useful to have since we'd want real node behaviour!
there is also`p2p_leak.py`, `p2p_timeouts.py`, `p2p_tx_privacy.py` where `on_version()` is used with `add_p2p_connection()`/inbound to `TestNode` connections which aren't affected by this. I guess we should think about it only if we end up doing outbound to `TestNode` connections there.
(https://github.com/bitcoin/bitcoin/pull/29353#issuecomment-1922955178)
tested ACK c340503b67585b631909584f1acaa327f5af25d3. very useful to have since we'd want real node behaviour!
there is also`p2p_leak.py`, `p2p_timeouts.py`, `p2p_tx_privacy.py` where `on_version()` is used with `add_p2p_connection()`/inbound to `TestNode` connections which aren't affected by this. I guess we should think about it only if we end up doing outbound to `TestNode` connections there.
💬 petertodd commented on issue "Cluster mempool, CPFP carveout, and V3 transaction policy":
(https://github.com/bitcoin/bitcoin/issues/29319#issuecomment-1923061666)
> @petertodd
>
> > Can you give a bit more detail on what challenges you think that'll pose?
>
> from my memory: "How this new replacement rule would behave if you have a parent in the "replace-by-feerate" half but the child is in the "replace-by-fee" one ?” see the conversation here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019839.html in past conversations we have assumed a “static” N blocks worth of mempool, unclear to me with your proposal if dynamic. i wond
...
(https://github.com/bitcoin/bitcoin/issues/29319#issuecomment-1923061666)
> @petertodd
>
> > Can you give a bit more detail on what challenges you think that'll pose?
>
> from my memory: "How this new replacement rule would behave if you have a parent in the "replace-by-feerate" half but the child is in the "replace-by-fee" one ?” see the conversation here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019839.html in past conversations we have assumed a “static” N blocks worth of mempool, unclear to me with your proposal if dynamic. i wond
...
💬 petertodd commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1923151784)
> I've heard similar arguments from many people, and I have to say I disagree. The miners need to follow the will of the users (especially the holders), or bitcoin cannot survive long-term. Mining pools are highly attuned to short-term interests, generally at the expense of long-term considerations. This is understandable given the competitive and unpredictable nature of proof-of-work pooled mining. A pool operator who tries to be a good citizen and caretake bitcoin's long-term interests will mo
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1923151784)
> I've heard similar arguments from many people, and I have to say I disagree. The miners need to follow the will of the users (especially the holders), or bitcoin cannot survive long-term. Mining pools are highly attuned to short-term interests, generally at the expense of long-term considerations. This is understandable given the competitive and unpredictable nature of proof-of-work pooled mining. A pool operator who tries to be a good citizen and caretake bitcoin's long-term interests will mo
...
💬 BitcoinMechanic commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1923175031)
The issue with (I hope) accidental conflation of "miners" with "pool operators" as you are doing @petertodd is that it makes a trivial task (getting a dozen people to flip a `1` to a `0`) seem insurmountable by implying that we are dealing with miners in general. We aren't.
As you point out, it is **extremely easy** to get three or four entities to do something like mempoolfullrbf=1.
The difference here is we aren't trying to route around network defaults, we are trying to have them be set
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1923175031)
The issue with (I hope) accidental conflation of "miners" with "pool operators" as you are doing @petertodd is that it makes a trivial task (getting a dozen people to flip a `1` to a `0`) seem insurmountable by implying that we are dealing with miners in general. We aren't.
As you point out, it is **extremely easy** to get three or four entities to do something like mempoolfullrbf=1.
The difference here is we aren't trying to route around network defaults, we are trying to have them be set
...
💬 petertodd commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1923231248)
> The issue with (I hope) accidental conflation of "miners" with "pool operators" as you are doing @petertodd is that it makes a trivial task (getting a dozen people to flip a `1` to a `0`) seem insurmountable by implying that we are dealing with miners in general. We aren't.
If it's so trivial, you should do that first: get a majority of hash power to set `-permitbaremultisig=0`. If you can, then you'd have a much better argument for this pull-req.
> As you point out, it is **extremely
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1923231248)
> The issue with (I hope) accidental conflation of "miners" with "pool operators" as you are doing @petertodd is that it makes a trivial task (getting a dozen people to flip a `1` to a `0`) seem insurmountable by implying that we are dealing with miners in general. We aren't.
If it's so trivial, you should do that first: get a majority of hash power to set `-permitbaremultisig=0`. If you can, then you'd have a much better argument for this pull-req.
> As you point out, it is **extremely
...
💬 BitcoinMechanic commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1923269102)
>If it's so trivial, you should do that first: get a majority of hash power to set -permitbaremultisig=0. If you can, then you'd have a much better argument for this pull-req.
Lobbying mining pools in order to backwards-rationalize and shoehorn changes in to bitcoin core itself is perhaps more efficient in that you get to take advantage of the awful centralization issue we have with pools currently, but is clearly not the correct way to go about things.
If you're going to assert that the c
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1923269102)
>If it's so trivial, you should do that first: get a majority of hash power to set -permitbaremultisig=0. If you can, then you'd have a much better argument for this pull-req.
Lobbying mining pools in order to backwards-rationalize and shoehorn changes in to bitcoin core itself is perhaps more efficient in that you get to take advantage of the awful centralization issue we have with pools currently, but is clearly not the correct way to go about things.
If you're going to assert that the c
...
💬 stratospher commented on pull request "test: make v2transport arg in addconnection mandatory and few cleanups":
(https://github.com/bitcoin/bitcoin/pull/29356#issuecomment-1923275862)
> Another possible alternative for addconnection's third parameter handling would be to take as default whatever is set by -v2transport, like we already do it for addnode (i.e. bool use_v2transport = self.MaybeArg<bool>(2).value_or(node_v2transport);).
@theStack, i went with the mandatory third parameter approach because:
1. this can be used only for interaction between python dummy peer(`P2PInterface`) and bitcoind(`TestNode`) on only regtest anyways.
2. we're indirectly passing whatever i
...
(https://github.com/bitcoin/bitcoin/pull/29356#issuecomment-1923275862)
> Another possible alternative for addconnection's third parameter handling would be to take as default whatever is set by -v2transport, like we already do it for addnode (i.e. bool use_v2transport = self.MaybeArg<bool>(2).value_or(node_v2transport);).
@theStack, i went with the mandatory third parameter approach because:
1. this can be used only for interaction between python dummy peer(`P2PInterface`) and bitcoind(`TestNode`) on only regtest anyways.
2. we're indirectly passing whatever i
...