Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 achow101 commented on issue "Implement PayJoin / Pay-to-EndPoint":
(https://github.com/bitcoin/bitcoin/issues/19148#issuecomment-3434484884)
BIP 78 will not be implemented because of the TLS requirement. However, I think we could implement BIP 77 Async Payjoin, and have opened a tracking/brainstorming issue for that: #33684
💬 scgbckbone commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#issuecomment-3434578564)
Thanks Rob. I completely forget about that 22nd bit to change to time-based lock.

To update my previous comment: anything without consensus meaning should not be allowed to import.

Only `older(1 - 65535)` & `older(4194305 - 4259839)` should be importable.
💬 DanGould commented on issue "Async Payjoin":
(https://github.com/bitcoin/bitcoin/issues/33684#issuecomment-3434583092)
There are a few questions that need to be answered for implementing Async Payjoin

> - If we are the Receiver, how do we get the pubkey of the Directory. Some suggestions are:
> - The user gets it out of band somehow

I think getting the keys from another receiver's URI was the simple out of band bootstrap mechanism @nothingmuch and I had in mind. The Payjoin URI shared by a receiver contains the OHTTP Key Config ("pubkey of the directory.") Cache that key and allow receiving to that director
...
💬 kevkevinpal commented on pull request "doc: Add blockman param to GetTransaction doc comment":
(https://github.com/bitcoin/bitcoin/pull/33683#discussion_r2453609626)
Just add the comment instead of changing the order of the params.

```suggestion
* @param[out] hashBlock The block hash, if the tx was found via -txindex or block_index
* @param[in] blockman Used to access and read blocks from disk
```
💬 kevkevinpal commented on pull request "doc: Add blockman param to GetTransaction doc comment":
(https://github.com/bitcoin/bitcoin/pull/33683#issuecomment-3434607168)
Since this is just a doc change and not a refactor I'd just move the comment down a line and only make a change there.
💬 musaHaruna commented on pull request "doc: Add blockman param to GetTransaction doc comment":
(https://github.com/bitcoin/bitcoin/pull/33683#discussion_r2454000772)
I followed the coding style from the documentation [coding-style-doxygen-compatible-comments](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-doxygen-compatible-comments). Maybe I might be overlooking something.
💬 maflcko commented on pull request "doc: Add blockman param to GetTransaction doc comment":
(https://github.com/bitcoin/bitcoin/pull/33683#issuecomment-3435406659)
lgtm ACK c5a0abb270f37ec391ce6e3ea5219584816adc54
💬 maflcko commented on pull request "ci: Retry image building once on failure":
(https://github.com/bitcoin/bitcoin/pull/33677#discussion_r2454108438)
Ah, nice catch. Removed the `check=False`.
💬 maflcko commented on pull request "ci: Retry image building once on failure":
(https://github.com/bitcoin/bitcoin/pull/33677#discussion_r2454108730)
I don't think I can. do you have a *working* diff I can push?
💬 maflcko commented on pull request "ci: Retry image building once on failure":
(https://github.com/bitcoin/bitcoin/pull/33677#discussion_r2454109165)
Yes, that is what the code here is already doing. I don't understand the suggestion, do you have a diff I can push?
💬 fanquake commented on pull request "ci: Drop libFuzzer from msan fuzz task":
(https://github.com/bitcoin/bitcoin/pull/33666#issuecomment-3435493495)
@dergoegge @marcofleon any opinion here?
💬 fanquake commented on pull request "ci: Doc ASLR workaround for sanitizer tasks":
(https://github.com/bitcoin/bitcoin/pull/33674#issuecomment-3435494398)
ACK fa0e36156cba535846ae2ecfaaa554d7f14fcdfd
fanquake closed an issue: "TSAN/MSAN fails with vm.mmap_rnd_bits=32 even with llvm 18.1.3"
(https://github.com/bitcoin/bitcoin/issues/30674)
🚀 fanquake merged a pull request: "ci: Doc ASLR workaround for sanitizer tasks"
(https://github.com/bitcoin/bitcoin/pull/33674)
💬 dergoegge commented on pull request "ci: Drop libFuzzer from msan fuzz task":
(https://github.com/bitcoin/bitcoin/pull/33666#issuecomment-3435511754)
I guess the only thing we're loosing here is that if there is a harness with no corpus we won't be doing our short "sanity" 60 second fuzz from scratch.

I know that it is possible to get libFuzzer running with MSan because I'm doing that in my own infra but that is an entirely different setup (e.g. the llvm version is different).

Perhaps in the future we can re-enable libFuzzer but for now I see no reason not to drop it, given that there are repeated annoying false-positives. Concept ACK.
💬 theuni commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3435534044)
Agreed. Thanks for having a look.
💬 stickies-v commented on pull request "doc: mention key removal in rpc interface modification":
(https://github.com/bitcoin/bitcoin/pull/32867#discussion_r2454210186)
Sorry, that was terrible suggestion. I wanted to keep the language consistent.

```suggestion
- It's preferable to avoid changing an RPC in a backward-incompatible manner, but in that case, add an associated `-deprecatedrpc=` option to retain previous RPC behavior during the deprecation period. Backward-incompatible changes include: data type changes (e.g. from `{"warnings":""}` to `{"warnings":[]}`, changing a value from a string to a number, etc.), logical meaning changes of a value, key n
...
💬 musaHaruna commented on pull request "doc: Add blockman param to GetTransaction doc comment":
(https://github.com/bitcoin/bitcoin/pull/33683#issuecomment-3435568362)
Friendly ping @TheCharlatan and @mzumsande
💬 maflcko commented on pull request "ci: Drop libFuzzer from msan fuzz task":
(https://github.com/bitcoin/bitcoin/pull/33666#issuecomment-3435574615)
Thx, added the sanity drop to the description.
🚀 fanquake merged a pull request: "depends: Avoid `warning: "_FORTIFY_SOURCE" redefined` for `libevent`"
(https://github.com/bitcoin/bitcoin/pull/32266)