Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 pstratem commented on pull request "Cache m_cached_finished_ibd where SetTip is called.":
(https://github.com/bitcoin/bitcoin/pull/32885#issuecomment-3114274259)
> @pstratem, not sure if you saw this, but could be helpful: #25081

That seems like it would be useful, but for now I'm just parsing the lock contention messages out of debug.log
📝 jbampton opened a pull request: "doc(src): fix typos"
(https://github.com/bitcoin/bitcoin/pull/33057)
Minor PR to fix spelling
maflcko closed a pull request: "doc(src): fix typos"
(https://github.com/bitcoin/bitcoin/pull/33057)
💬 maflcko commented on pull request "doc(src): fix typos":
(https://github.com/bitcoin/bitcoin/pull/33057#issuecomment-3114286903)
thx, but this is the wrong repo. you'll have to submit upstream
💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2229151399)
In `SQLiteStatement::Column<T>()`, one of the ways we construct the appropriate data from a blob is to do `reinterpret_cast<T::value_type>`. `Txid` met all of the requirements for using that constructor, except that it didn't have a `value_type`, hence this commit adding it.

But it looks like I didn't include that commit in this PR. Rather, it's https://github.com/bitcoin/bitcoin/pull/33034/commits/afe99b3b7df6c658fb4068c6418dfd7ebf151460 in #33034.
🚀 glozow merged a pull request: "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC"
(https://github.com/bitcoin/bitcoin/pull/32593)
💬 achow101 commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2229152163)
Hmm, yeah, I guess we don't really need it.
💬 ryanofsky commented on pull request "bitcoin-cli: Add -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/32297#issuecomment-3114315569)
re: https://github.com/bitcoin/bitcoin/pull/32297#issuecomment-3074628613

Thanks for the report! I opened an issue to track this in https://github.com/bitcoin-core/libmultiprocess/issues/191 and I suspect the changes in https://github.com/bitcoin/bitcoin/pull/32345 could fix this or at least make problem easier to debug.
💬 w0xlt commented on pull request "wallet: Remove wallet version and several legacy related functions":
(https://github.com/bitcoin/bitcoin/pull/32977#discussion_r2229183945)
Reverted the PR to the previous version
📝 yancyribbens opened a pull request: "test: add assertions to SRD max weight test"
(https://github.com/bitcoin/bitcoin/pull/33058)
Replace generic assertion with a result specific assertion showing the correctness of the solution found. If the max weight parameter is exceeded, the least valuable `UTXOs` are removed from the result. Therefore, only the most valued _encountered_ `UTXO's` are selected. While the smallest set would include all the most valued `UTXO's`, in the case of the test there is one high value `UTXO` that is never found before the target value is reached.

Correct the test comment to be more specific a
...
achow101 closed a pull request: "wallet: Set minversion to FEATURE_LATEST during migration"
(https://github.com/bitcoin/bitcoin/pull/33041)
💬 yancyribbens commented on pull request "test: add assertions to SRD max weight test":
(https://github.com/bitcoin/bitcoin/pull/33058#issuecomment-3114358388)
cc @murchandamus
💬 l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2229169977)
Slightly unrelated question: given that `WalletLogPrintf` calls `LogInfo` now
* https://github.com/bitcoin/bitcoin/blob/0f86da382d3fdca6fc69ff277794acbc3f1e928d/src/wallet/wallet.h#L940-L943

(which was changed in https://github.com/bitcoin/bitcoin/commit/facbcd4cef8890ae18976fb53b67ea56b3c04454#diff-bf36b7865eb69d4b0a3576d830004933c82a795d3fd73c8073c11fc2e637a5c4R259-R261), I was wondering if this is the right time to rename the method as well. I don't mind if you think this isn't the place
...
💬 l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2229196695)
nit, if you touch again, please consider adding braces here to avoid confusion
💬 l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2229201811)
nit: the commit message claims `This refactor does not change behavior`, but we're removing a redundant space here, which could theoretically invalidate previous log parsers. It seems fine to me to do that, but we might want to mention it in the commit message.
💬 l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2229178343)
> The whole point of having log levels is that they are used correctly

I wasn't recommending any incorrect usage, was just wondering if it would simplify things if we migrated the warning/error logs before this PR, so that we can automate migration of all remaining ones and remove the `LogInfo` alias here. I'm also fine with it as is.

> If we do care, we should just review it

I'm not against this change, I just don't fully understand why some of the `LogPrintf` weren't migrated, e.g.
*
...
👍 l0rinc approved a pull request: "log: [refactor] Use info level for init logs"
(https://github.com/bitcoin/bitcoin/pull/32967#pullrequestreview-3052616273)
ACK fa4002b3534fbd22a43b32cb24420652d12da6d2
🤔 mzumsande reviewed a pull request: "net, validation: don't punish peers for consensus-invalid txs"
(https://github.com/bitcoin/bitcoin/pull/33050#pullrequestreview-3052706227)
> Because we do not discourage nodes for transactions we consider non-standard, we don't get any DoS protection from this check in adversarial scenarios.

Could you expand on the logic of this - is it just because the most effective DoS vectors known today happen to involve non-standard but valid transactions, or does this hold with more generality?
What if the current DoS vectors involving non-standard txs are fixed in the future, and the most-effective known DoS vectors then will involve co
...
👋 achow101's pull request is ready for review: "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet"
(https://github.com/bitcoin/bitcoin/pull/32489)
💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#issuecomment-3114420072)
All prerequisite PRs merged, ready for review.