✅ MarcoFalke closed a pull request: "[src/{core_read,core_write,psbt,txorphanage}.cpp] Use correct `size_type` for `std::vector` iteration"
(https://github.com/bitcoin/bitcoin/pull/27805)
(https://github.com/bitcoin/bitcoin/pull/27805)
💬 theuni commented on pull request "depends: modernize clang flags for Darwin":
(https://github.com/bitcoin/bitcoin/pull/27798#issuecomment-1574102337)
@fanquake and @hebasto have both pointed out to me in discussions that this works even without the `-nostdlibinc`. That is _purely_ accidental and coincidental, as the addition of `-nostdlibinc` is what's _intended_ to make the other changes here work.
By sheer luck though, they manage to put everything in the correct order such that the poisoned paths come last in the search-order. That feels brittle, but maybe good enough.
I'd also like to see what the consequences of the `argument unuse
...
(https://github.com/bitcoin/bitcoin/pull/27798#issuecomment-1574102337)
@fanquake and @hebasto have both pointed out to me in discussions that this works even without the `-nostdlibinc`. That is _purely_ accidental and coincidental, as the addition of `-nostdlibinc` is what's _intended_ to make the other changes here work.
By sheer luck though, they manage to put everything in the correct order such that the poisoned paths come last in the search-order. That feels brittle, but maybe good enough.
I'd also like to see what the consequences of the `argument unuse
...
💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1214670321)
oof. some of the spacing is not quite right here even for cascading. I'm going to fix it in the new compilation unit/file, maybe then it will make a little more sense? I'm worried lining them up isn't going to be much help. It's hard to read too because the declaration for this 4D array happens by nesting arrays in arrays :sob:
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1214670321)
oof. some of the spacing is not quite right here even for cascading. I'm going to fix it in the new compilation unit/file, maybe then it will make a little more sense? I'm worried lining them up isn't going to be much help. It's hard to read too because the declaration for this 4D array happens by nesting arrays in arrays :sob:
📝 Xekyo opened a pull request: "fuzz: Fix mini_miner_selection running out of coin"
(https://github.com/bitcoin/bitcoin/pull/27806)
Fixes a bug in the mini_miner_selection fuzz test found by fuzzing: It was possible for the mini_miner_selection fuzz test to generated transactions that created fewer new outputs than the two inputs they each spent. If the fuzz seed did so consistently, eventually it would cause a `pop_front()` on an empty available_coins which resulted in undefined behavior.
Fixed per belt-suspender approach:
- assert that available_coins is not empty before generating tx
- generate at least two coins per
...
(https://github.com/bitcoin/bitcoin/pull/27806)
Fixes a bug in the mini_miner_selection fuzz test found by fuzzing: It was possible for the mini_miner_selection fuzz test to generated transactions that created fewer new outputs than the two inputs they each spent. If the fuzz seed did so consistently, eventually it would cause a `pop_front()` on an empty available_coins which resulted in undefined behavior.
Fixed per belt-suspender approach:
- assert that available_coins is not empty before generating tx
- generate at least two coins per
...
💬 sr-gi commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1574166059)
> > What's the rationale regarding optimizing for memory instead of privacy here? I can see how this simplifies the design, however, I fail to see this being an obvious pick given we'd be potentially even more exposed to fingerprinting than before (as is now it is trivial for every connection to probe data on the node's mempool).
>
> This isn't trading off privacy -- that's the point of [this comment](https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1549711784). It does mean that so
...
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1574166059)
> > What's the rationale regarding optimizing for memory instead of privacy here? I can see how this simplifies the design, however, I fail to see this being an obvious pick given we'd be potentially even more exposed to fingerprinting than before (as is now it is trivial for every connection to probe data on the node's mempool).
>
> This isn't trading off privacy -- that's the point of [this comment](https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1549711784). It does mean that so
...
💬 achow101 commented on pull request "wallet: Add tracing for sqlite statements":
(https://github.com/bitcoin/bitcoin/pull/27801#issuecomment-1574167988)
Turning this on will log private keys as they are written to the db, which seems a little scary.
(https://github.com/bitcoin/bitcoin/pull/27801#issuecomment-1574167988)
Turning this on will log private keys as they are written to the db, which seems a little scary.
⚠️ kevkevinpal opened an issue: "depriortisetransaction"
(https://github.com/bitcoin/bitcoin/issues/27807)
### Please describe the feature you'd like to see added.
If a user had already prioritized a transaction to be mined there would be no way to get it deprioritized until after this PR
https://github.com/bitcoin/bitcoin/pull/27501 which adds a new RPC call to getpriotisationmap. There should be a way to remove the prioritization entirely and set it to zero without calling two separate RPCs.
### Is your feature related to a problem, if so please describe it.
If a miner wants to deprioritize
...
(https://github.com/bitcoin/bitcoin/issues/27807)
### Please describe the feature you'd like to see added.
If a user had already prioritized a transaction to be mined there would be no way to get it deprioritized until after this PR
https://github.com/bitcoin/bitcoin/pull/27501 which adds a new RPC call to getpriotisationmap. There should be a way to remove the prioritization entirely and set it to zero without calling two separate RPCs.
### Is your feature related to a problem, if so please describe it.
If a miner wants to deprioritize
...
💬 pinheadmz commented on issue "depriortisetransaction":
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574169936)
rpc `prioritizetransaction` can be called with a negative delta already, right?
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574169936)
rpc `prioritizetransaction` can be called with a negative delta already, right?
💬 kevkevinpal commented on issue "depriortisetransaction":
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574172151)
> rpc `prioritizetransaction` can be called with a negative delta already, right?
Yea exactly but that requires the user to track the txid and its delta or after https://github.com/bitcoin/bitcoin/pull/27501 is merged track the txid call getpriotisationmap and then use the negative delta from there
was thinking it would be easier if the user could just specify the txid they want to remove
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574172151)
> rpc `prioritizetransaction` can be called with a negative delta already, right?
Yea exactly but that requires the user to track the txid and its delta or after https://github.com/bitcoin/bitcoin/pull/27501 is merged track the txid call getpriotisationmap and then use the negative delta from there
was thinking it would be easier if the user could just specify the txid they want to remove
💬 willcl-ark commented on issue "depriortisetransaction":
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574177098)
`getmempoolentry` RPC also shows you the modified fee, which includes the priotirization delta (although still requires reverse calculation).
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574177098)
`getmempoolentry` RPC also shows you the modified fee, which includes the priotirization delta (although still requires reverse calculation).
💬 pinheadmz commented on issue "depriortisetransaction":
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574177955)
> was thinking it would be easier if the user could just specify the txid they want to remove
can't you just call prioritisetransaction with txid and, like, `-999999` ?
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574177955)
> was thinking it would be easier if the user could just specify the txid they want to remove
can't you just call prioritisetransaction with txid and, like, `-999999` ?
💬 kevkevinpal commented on issue "depriortisetransaction":
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574181505)
> > was thinking it would be easier if the user could just specify the txid they want to remove
>
> can't you just call prioritisetransaction with txid and, like, `-999999` ?
wouldn't this make the map delta never include that txid, maybe depriortisetransaction isn't the best naming but instead removing any preference on a txid. Removing it from the mapdeltas. Goal is to set the mapdelta for a txid to 0.
maybe `removepriortisation` instead?
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574181505)
> > was thinking it would be easier if the user could just specify the txid they want to remove
>
> can't you just call prioritisetransaction with txid and, like, `-999999` ?
wouldn't this make the map delta never include that txid, maybe depriortisetransaction isn't the best naming but instead removing any preference on a txid. Removing it from the mapdeltas. Goal is to set the mapdelta for a txid to 0.
maybe `removepriortisation` instead?
💬 kevkevinpal commented on issue "depriortisetransaction":
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574182072)
> `getmempoolentry` RPC also shows you the modified fee, which includes the priotirization delta (although still requires reverse calculation).
didn't know that thanks, but ya still reverse calculation still required
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574182072)
> `getmempoolentry` RPC also shows you the modified fee, which includes the priotirization delta (although still requires reverse calculation).
didn't know that thanks, but ya still reverse calculation still required
💬 pinheadmz commented on issue "depriortisetransaction":
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574182839)
> maybe `removepriortisation` instead?
GOTCHA, well maybe instead of a new rpc, maybe `priotirisetransaction <txid> 0` could be a magic word for reset like you want
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1574182839)
> maybe `removepriortisation` instead?
GOTCHA, well maybe instead of a new rpc, maybe `priotirisetransaction <txid> 0` could be a magic word for reset like you want
🤔 pinheadmz reviewed a pull request: "Relay own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/27509#pullrequestreview-1458209334)
concept ACK
Reviewed all code - really great job with this! Very cool idea. I will continue to track this PR and look forward to testing it out more and trying to break the functional tests ;-)
I have a few questions below about implementation
(https://github.com/bitcoin/bitcoin/pull/27509#pullrequestreview-1458209334)
concept ACK
Reviewed all code - really great job with this! Very cool idea. I will continue to track this PR and look forward to testing it out more and trying to break the functional tests ;-)
I have a few questions below about implementation
💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214690639)
Just so I understand: this change is required because with the PR we wouldn't send our own TX in response to a `MEMPOOL` message, so you quickly spin up a dummy peer to send the TX to us first. Then we can assert that it was relayed to `fitler_peer` ?
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214690639)
Just so I understand: this change is required because with the PR we wouldn't send our own TX in response to a `MEMPOOL` message, so you quickly spin up a dummy peer to send the TX to us first. Then we can assert that it was relayed to `fitler_peer` ?
💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214675802)
Why is this extra ping needed for sensitive connections ?
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214675802)
Why is this extra ping needed for sensitive connections ?
💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214698071)
Super cool idea! Is there common chance for failure here though? This relies on a chain of peers from the sensitive relay node back to us that haven't seen the tx yet.
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214698071)
Super cool idea! Is there common chance for failure here though? This relies on a chain of peers from the sensitive relay node back to us that haven't seen the tx yet.
💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214671917)
Why use a funny value here instead of our actual subver? I'm guessing it's for extra anonymity? Could there be any downsides though, like nodes that refuse connections to certain agents?
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214671917)
Why use a funny value here instead of our actual subver? I'm guessing it's for extra anonymity? Could there be any downsides though, like nodes that refuse connections to certain agents?
💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214678210)
Why disconnect the sensitive connection on PONG? Could that ever happen before we send our tx messages out?
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214678210)
Why disconnect the sensitive connection on PONG? Could that ever happen before we send our tx messages out?
💬 pinheadmz commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214696791)
nit: don't we usually do this kind of logic in one line?
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1214696791)
nit: don't we usually do this kind of logic in one line?