Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 achow101 commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2085813449)
Migrate is a term that is used in other computer related contexts, e.g. "database migration". It was chosen because of the process's similarities to a database migration, and particularly in a way similar to how Django [uses it](https://docs.djangoproject.com/en/5.0/topics/migrations/). Since Django has similar usage of the term with translations, perhaps we can point translators to there?

While "upgrade" was considered, we're already using that to mean something else, and I don't think overl
...
💬 theStack commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1585154706)
Ugh indeed, apparently I've used a word for several years without noticing that it doesn't exist (mostly in "ephermal port") :D fixed.
💬 theStack commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1585154931)
> [901ffa9](https://github.com/bitcoin/bitcoin/commit/901ffa9e6e57f7c0e0db967fba9c715139bbcf0b) shouldn't this be sent from the ephemeral miniwallet?

If the to-be-evicted tx and the filling txs originate from the same miniwallet instance, the problem fixed by e9dc511a7e9a562f953ff93f358102f555f583e6 is unfortunately re-introduced, as it could happen again that a filler tx spends from the to-be-evicted tx, increasing its' effective fee-rate and thus avoiding the expected eviction (making the a
...
🚀 achow101 merged a pull request: "Disable util::Result copying and assignment"
(https://github.com/bitcoin/bitcoin/pull/29906)
💬 laanwj commented on pull request "JSON-RPC request Content-Type is application/json":
(https://github.com/bitcoin/bitcoin/pull/29946#issuecomment-2085872620)
Code review ACK f90a84d61505443fd3bb83253c091590b3dc7f45
💬 ryanofsky commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2085898761)
Thanks, I added a link directly to this comment in the summary so those who are interested can follow the chain of reasoning.

To reiterate my own point of view, `-logsourcelocation` does not solve the same problem this PR solves because it does not (1) let libitcoinkernel applications call validation functions and control which `Logger` object they write to, and (2) allow code like wallet code and index code to easily include additional metadata in log messages (wallet names and index names)
...
👍 MarnixCroes approved a pull request: "gui: fix misleading signmessage error with segwit"
(https://github.com/bitcoin-core/gui/pull/819#pullrequestreview-2031988964)
lgtm 83def1c5a3741878aa63e6f28cc3def99f76c358
useful clarification
💬 brunoerg commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1585186363)
Not 100% related to these change, but`fill_mempool` requires -datacarriersize=100000 and -maxmempool=5 and in `rpc_packages` test, this is mentioned:
```py
# Make chain of two transactions where parent doesn't make minfee threshold
# but child is too high fee
# Lower mempool limit to make it easier to fill_mempool
self.restart_node(0, extra_args=[
"-datacarriersize=100000",
"-maxmempool=5",
"-persistmempool=0",
])
```

Perhaps, it would worth to also mention it in `mempoo
...
💬 sipa commented on pull request "msvc: Compile `test\fuzz\bitdeque.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29983#issuecomment-2085961219)
utACK 774359b4a96d2724dc70f900cb71e084a77164da
💬 theStack commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2086022405)
Concept ACK
💬 dongcarl commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086032180)
If I remember correctly, I think that you can likely just patch OpenSSL here since it's not a package that is "core" to Guix like GnuTLS was?
👍 pinheadmz approved a pull request: "refactor: Avoid unused-variable warning in init.cpp"
(https://github.com/bitcoin/bitcoin/pull/29968#pullrequestreview-2032047471)
ACK fa2353d9c3ff68995e7478b9b80dcdb1fc638a53

Confirmed the warning on master by setting `-Wall` and then forcing `HAVE_SOCKADDR_UN = 0` in `configure.ac`. This patch fixes the warning in a clean way. I dunno if `(void)unix;` needs an explanatory comment or if thats a recognizable pattern for this kind of thing.

Thanks again @maflcko for cleaning up my mess 😬

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK fa2353d9c3ff68995e747
...
👍 BrandonOdiwuor approved a pull request: "gui: fix misleading signmessage error with segwit"
(https://github.com/bitcoin-core/gui/pull/819#pullrequestreview-2032085316)
Code Review ACK 83def1c5a3741878aa63e6f28cc3def99f76c358
👍 hebasto approved a pull request: "refactor: Avoid unused-variable warning in init.cpp"
(https://github.com/bitcoin/bitcoin/pull/29968#pullrequestreview-2032098076)
re-ACK fa2353d9c3ff68995e7478b9b80dcdb1fc638a53.
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2086222091)
rebased on master due to conflict from https://github.com/bitcoin/bitcoin/pull/29906
💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086252348)
I wrote:

> we don't want to bump our Guix Time Machine commit just for that.

Actually our Time Machine commit much more recent than I thought. Will defer to @dongcarl.
💬 maflcko commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086320606)
> > we don't want to bump our Guix Time Machine commit just for that.
>
> Actually our Time Machine commit is much more recent than I thought.

I presume openssl is used in the bootstrap chain, to bootstrap older software, so it probably can never be removed in a time machine bump?
💬 maflcko commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086326853)
> If I remember correctly, I think that you can likely just patch OpenSSL here since it's not a package that is "core" to Guix like GnuTLS was?

Yes, this is workaround 3.

> "Workaround 3: Disable the tests in the Guix source code for this single derivation"
📝 pinheadmz opened a pull request: "test: use sleepy wait-for-log in reindex readonly"
(https://github.com/bitcoin/bitcoin/pull/30006)
Also rename the busy wait-for-log method to prevent recurrence. See https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1532578152