Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381557)
done
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381462)
done
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381283)
done
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381691)
done
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1452381645)
done
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1892185801)
Thanks for the review @andrewtoth!
All nits tackled, [small diff](https://github.com/bitcoin/bitcoin/compare/6c603490023317a84c7c96ef4b64f4f96ea03d1f..79e10351b1ce1f8db98ece67aacc24f323008b37). Ready to go.
💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1892249295)
Rebased on fix for CI issue
:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/29250)
💬 fanquake commented on pull request "build: remove `--enable-lto`":
(https://github.com/bitcoin/bitcoin/pull/29185#issuecomment-1892283371)
This is waiting on https://github.com/google/oss-fuzz/pull/11498 downstream.
🤔 furszy reviewed a pull request: "sqlite: Disallow writing from multiple `SQLiteBatch`s"
(https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1821862931)
> Changed TxnAbort and TxnCommit to always release the lock.

This breaks the db txn isolation property.
The db txn is still active at the engine level, the lock shouldn't be released when the sqlite statement fails to execute (only when the abort/commit command executes successfully). Otherwise, other concurrent handlers are allowed to access db at the same time and, in the worst possible outcome, dump information to disk that should have been discarded.

Check this out 71170998. Made a te
...
💬 maflcko commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1452493222)
are you still working on this?
🚀 fanquake merged a pull request: "log mempool loading progress"
(https://github.com/bitcoin/bitcoin/pull/29227)
💬 achow101 commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#issuecomment-1892439363)
> The db txn is still active at the engine level, the lock shouldn't be released when the sqlite statement fails to execute (only when the abort/commit command executes successfully). Otherwise, other concurrent handlers are allowed to access db at the same time and, in the worst possible outcome, dump information to disk that should have been discarded.

If it fails to abort when the batch is destructed, then we're deadlocked. I don't see how this can be resolved without deadlocking and still
...
fanquake closed a pull request: "doc: update encryptwallet passphrase doc"
(https://github.com/bitcoin/bitcoin/pull/29245)
💬 fanquake commented on pull request "doc: update encryptwallet passphrase doc":
(https://github.com/bitcoin/bitcoin/pull/29245#issuecomment-1892449257)
> Follow up to https://github.com/bitcoin/bitcoin/pull/28974, coming from https://github.com/bitcoin/bitcoin/pull/28974#issuecomment-1855661692

Given that #28974 has not been merged, you can just make any further changes, have further discussion in that PR. Not sure about adding all this text. In any case, you'll need to at least remove the markdown formatting.
📝 fanquake converted_to_draft a pull request: "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us"
(https://github.com/bitcoin/bitcoin/pull/29145)
To avoid issues with DNS blacklisting, I've setup a separate domain for my DNS seed.

Like #28936

I've chosen a domain name that is explicitly verbose about its purpose and the possibility of malware on resolved IPs, to go an extra mile in helping avoid any attempts to abuse it.
💬 fanquake commented on pull request "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us":
(https://github.com/bitcoin/bitcoin/pull/29145#issuecomment-1892470188)
> but if there's a hard objection to it, I can come up with another domain for it.

I'd say objections to your current choice of domain name have been made clear from various contributors. Along with objection to changing any logging to accomodate it. Moved this to draft for now. Feel-free to undraft with a different domain etc.
📝 TheCharlatan opened a pull request: "contrib: Add --binary option to clang-format-diff"
(https://github.com/bitcoin/bitcoin/pull/29251)
This was adapted from https://github.com/llvm-mirror/clang/commit/145510a469c6efd18bd98cd3c3f306492a9af90d and is useful for systems where clang tools are shipped with a version suffix.