💬 stickies-v commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2210416125)
nit: would be good to add braces or move to one-liner while touching this
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2210416125)
nit: would be good to add braces or move to one-liner while touching this
💬 stickies-v commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2210405561)
I think `LogWarning` could be appropriate here?
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2210405561)
I think `LogWarning` could be appropriate here?
👋 brunoerg's pull request is ready for review: "fuzz: wallet: add target for tx scanning"
(https://github.com/bitcoin/bitcoin/pull/32993)
(https://github.com/bitcoin/bitcoin/pull/32993)
💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3078633756)
Concept ACK bb14e8ac7d38f3698d76ea3d6dce09e1387d59f3
Started reviewing the PR.
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3078633756)
Concept ACK bb14e8ac7d38f3698d76ea3d6dce09e1387d59f3
Started reviewing the PR.
💬 instagibbs commented on pull request "validation: docs and cleanups for MemPoolAccept coins views":
(https://github.com/bitcoin/bitcoin/pull/32973#discussion_r2210398185)
> The view doesn't track whether a coin previously existed but has now been spent
been spent in mempool/package contexts, I presume
(https://github.com/bitcoin/bitcoin/pull/32973#discussion_r2210398185)
> The view doesn't track whether a coin previously existed but has now been spent
been spent in mempool/package contexts, I presume
👍 instagibbs approved a pull request: "validation: docs and cleanups for MemPoolAccept coins views"
(https://github.com/bitcoin/bitcoin/pull/32973#pullrequestreview-3022450569)
ACK b5805eddefced4cd8bb6f8069ea0a4bf199bacff
confirmed that the remaining tests cases fail when specific key clauses are modified, that it seems implausible that the test adds much assurances, and honestly is not a particularly maintainable test. The new comments are helpful for me to trace the code in a directed way and confirm what it's saying.
(https://github.com/bitcoin/bitcoin/pull/32973#pullrequestreview-3022450569)
ACK b5805eddefced4cd8bb6f8069ea0a4bf199bacff
confirmed that the remaining tests cases fail when specific key clauses are modified, that it seems implausible that the test adds much assurances, and honestly is not a particularly maintainable test. The new comments are helpful for me to trace the code in a directed way and confirm what it's saying.
📝 danielabrozzoni opened a pull request: "doc: clarify AddrMan::GetAddresses documentation"
(https://github.com/bitcoin/bitcoin/pull/32994)
Improve the GetAddresses documentation by specifying that one version of the function uses the address response cache while the other does not.
Also update the outdated "call the function without a parameter" phrasing in the cached version. This wording was accurate when the cache was introduced in #18991, but became outdated after later commits (f26502e9fc8a669b30717525597e3f468eaecf79, 81b00f87800f40cb14f2131ff27668bd2bb9e551) added parameters to each function.
(https://github.com/bitcoin/bitcoin/pull/32994)
Improve the GetAddresses documentation by specifying that one version of the function uses the address response cache while the other does not.
Also update the outdated "call the function without a parameter" phrasing in the cached version. This wording was accurate when the cache was introduced in #18991, but became outdated after later commits (f26502e9fc8a669b30717525597e3f468eaecf79, 81b00f87800f40cb14f2131ff27668bd2bb9e551) added parameters to each function.
💬 instagibbs commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078738029)
@kurapika007
> So why not increase it to 10 sats/vB—reducing bandwidth consumption by 10×, making the network 10× more resilient to spam, and sending the right economic signal for long-term sustainability?
Because especially with arbitrary values, the onus is on the person attempting to change it. Constantly fiddling with the default value is also not really a sustainable option even if we somehow found a new "optimal". If we enter a 90% drawdown bear market, we aren't going to revisit th
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078738029)
@kurapika007
> So why not increase it to 10 sats/vB—reducing bandwidth consumption by 10×, making the network 10× more resilient to spam, and sending the right economic signal for long-term sustainability?
Because especially with arbitrary values, the onus is on the person attempting to change it. Constantly fiddling with the default value is also not really a sustainable option even if we somehow found a new "optimal". If we enter a 90% drawdown bear market, we aren't going to revisit th
...
💬 maflcko commented on issue "getbestblockhash is sometimes taking a very long time":
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3078781773)
Does `-forcecompactdb` improve the situation for you?
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3078781773)
Does `-forcecompactdb` improve the situation for you?
💬 martinatime commented on issue "getbestblockhash is sometimes taking a very long time":
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3078804148)
I don't think I have tried that. Is there a way I can tell if the db needs to be compacted before I attempt this?
(https://github.com/bitcoin/bitcoin/issues/32733#issuecomment-3078804148)
I don't think I have tried that. Is there a way I can tell if the db needs to be compacted before I attempt this?
💬 sr-gi commented on pull request "Adds transaction propagation information to mempool transactions":
(https://github.com/bitcoin/bitcoin/pull/32986#issuecomment-3078827018)
> > I'm unsure whether the added complexity is justified, given that it's unlikely to be used beyond that specific context.
>
> Would adding tracepoint(s) be a suitable alternative? More overhead for the user, but probably much less in this repo?
I think existing tracepoints can be used as the base to build something similar outside Core, like `net:outbound_message` and `net:inbound_message`. Similarly, a log grabber (grepper?) can also work in some cases. The issue I see with both solutio
...
(https://github.com/bitcoin/bitcoin/pull/32986#issuecomment-3078827018)
> > I'm unsure whether the added complexity is justified, given that it's unlikely to be used beyond that specific context.
>
> Would adding tracepoint(s) be a suitable alternative? More overhead for the user, but probably much less in this repo?
I think existing tracepoints can be used as the base to build something similar outside Core, like `net:outbound_message` and `net:inbound_message`. Similarly, a log grabber (grepper?) can also work in some cases. The issue I see with both solutio
...
💬 kurapika007 commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078830847)
> @kurapika007
>
> Because especially with arbitrary values, the onus is on the person attempting to change it. Constantly fiddling with the default value is also not really a sustainable option even if we somehow found a new "optimal". If we enter a 90% drawdown bear market, we aren't going to revisit this value and raise it, are we?
>
Perhaps the solution lies not simply in arbitrarily adjusting technical parameters, but rather in adopting rules that better align the economic incentives of
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078830847)
> @kurapika007
>
> Because especially with arbitrary values, the onus is on the person attempting to change it. Constantly fiddling with the default value is also not really a sustainable option even if we somehow found a new "optimal". If we enter a 90% drawdown bear market, we aren't going to revisit this value and raise it, are we?
>
Perhaps the solution lies not simply in arbitrarily adjusting technical parameters, but rather in adopting rules that better align the economic incentives of
...
👍 TheCharlatan approved a pull request: "init: [gui] Avoid UB/crash in InitAndLoadChainstate"
(https://github.com/bitcoin/bitcoin/pull/32987#pullrequestreview-3025283492)
Thanks for fixing, should have thought of this when I started making the destructors more powerful! ACK on the fixes, the functional test is a bit hacky, but I don't have a better idea either. Maybe we could hook into the ui interface instead, but that's not necessarily cleaner and we only have one caller of `ThreadSafeQuestion` anyway.
(https://github.com/bitcoin/bitcoin/pull/32987#pullrequestreview-3025283492)
Thanks for fixing, should have thought of this when I started making the destructors more powerful! ACK on the fixes, the functional test is a bit hacky, but I don't have a better idea either. Maybe we could hook into the ui interface instead, but that's not necessarily cleaner and we only have one caller of `ThreadSafeQuestion` anyway.
💬 darosior commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078906634)
> If we enter a 90% drawdown bear market, we aren't going to revisit this value and raise it, are we?
Plus, we wouldn't be able to at all for software that is already released. Better have a conservative value that is resistant to an asset price decrease than an optimistic value that tries to captivate very low fees in times of high asset prices and low usage.
I am not against lowering the minrelay value, especially if it is shown that there is sustained usage of sub-1sat/vb transactions t
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078906634)
> If we enter a 90% drawdown bear market, we aren't going to revisit this value and raise it, are we?
Plus, we wouldn't be able to at all for software that is already released. Better have a conservative value that is resistant to an asset price decrease than an optimistic value that tries to captivate very low fees in times of high asset prices and low usage.
I am not against lowering the minrelay value, especially if it is shown that there is sustained usage of sub-1sat/vb transactions t
...
💬 sstone commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2210633065)
Correct, I did not want to loop twice over the block's transactions to compute the actual number of spent inputs.
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2210633065)
Correct, I did not want to loop twice over the block's transactions to compute the actual number of spent inputs.
💬 sstone commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2210635923)
How can I find the tx without the block's offset ?
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2210635923)
How can I find the tx without the block's offset ?
💬 l0rinc commented on pull request "[IBD] log start of script validation past `assumevalid` block":
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2210648124)
Many users are surprised by the slowdowns that suddenly happen during IBD, this should clarify that. If this doesn't work for some corner cases, I think that should be fine, it's just meant to help explain the shift in IBD performance.
I can change it to include the case when script verification starts initially, but I explicitly meant this log to signal when the effect of assumevalid gets turned off, i.e. to explain "grinding to a halt" that many users were reporting.
> See also https://g
...
(https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2210648124)
Many users are surprised by the slowdowns that suddenly happen during IBD, this should clarify that. If this doesn't work for some corner cases, I think that should be fine, it's just meant to help explain the shift in IBD performance.
I can change it to include the case when script verification starts initially, but I explicitly meant this log to signal when the effect of assumevalid gets turned off, i.e. to explain "grinding to a halt" that many users were reporting.
> See also https://g
...
💬 Rob1Ham commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078947536)
A naive question I have is how does this relate to another policy change like mempoolfullrbf? We currently have 20% of hashrate now mining sub 1 sat/vbyte ([source](https://x.com/mononautical/status/1944942585384198259)) and some mining pools are leaving revenue on the table due to not having the right mempool policy ([source](https://x.com/mononautical/status/1945328731821892043)), couldn't the argument be made that some mining pools are being advantaged by using the default values?
A fair c
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3078947536)
A naive question I have is how does this relate to another policy change like mempoolfullrbf? We currently have 20% of hashrate now mining sub 1 sat/vbyte ([source](https://x.com/mononautical/status/1944942585384198259)) and some mining pools are leaving revenue on the table due to not having the right mempool policy ([source](https://x.com/mononautical/status/1945328731821892043)), couldn't the argument be made that some mining pools are being advantaged by using the default values?
A fair c
...
🤔 furszy reviewed a pull request: "wallet: Remove `upgradewallet` RPC"
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3025398994)
It seems we're already thinking about upgrades in #32895 ?.
If that's the case, I'd rather keep the RPC. The maintenance cost should be minimal.
(https://github.com/bitcoin/bitcoin/pull/32944#pullrequestreview-3025398994)
It seems we're already thinking about upgrades in #32895 ?.
If that's the case, I'd rather keep the RPC. The maintenance cost should be minimal.
🤔 stickies-v reviewed a pull request: "Adds transaction propagation information to mempool transactions"
(https://github.com/bitcoin/bitcoin/pull/32986#pullrequestreview-3025461492)
I'm generally not in favour of making bespoke changes to production code or extending the public interface just to facilitate tests, monitoring, ... if there exists a workaround, especially for temporary projects. In this case, after speaking with @sr-gi offline a bit more, it seems it seems like tracepoints, logging, small patchsets, ... are feasible alternatives, so I'm Concept NACK on this one. (but I am open to making minimal changes to ensure the necessary erlay simulations can be ran)
(https://github.com/bitcoin/bitcoin/pull/32986#pullrequestreview-3025461492)
I'm generally not in favour of making bespoke changes to production code or extending the public interface just to facilitate tests, monitoring, ... if there exists a workaround, especially for temporary projects. In this case, after speaking with @sr-gi offline a bit more, it seems it seems like tracepoints, logging, small patchsets, ... are feasible alternatives, so I'm Concept NACK on this one. (but I am open to making minimal changes to ensure the necessary erlay simulations can be ran)