Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 jonatack opened a pull request: "rpc, util: avoid string copies in ParseHash/ParseHex functions"
(https://github.com/bitcoin/bitcoin/pull/28172)
These utility methods are called by quite a few RPCs and tests, as well as by each other.

```
$ git grep "ParseHashV\|ParseHashO\|ParseHexV\|ParseHexO" | wc -l
61
```

Also remove an out-of-date external link.
💬 achow101 commented on pull request "wallet: bugfix, disallow migration of invalid scripts":
(https://github.com/bitcoin/bitcoin/pull/28125#discussion_r1276759711)
Perhaps check that this label doesn't appear in any of the migrated wallets?
💬 luke-jr commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r1276765179)
Could just as well still be unsupported, not invalid.
💬 jonatack commented on pull request "refactor: Revert additional univalue check in ParseSighashString":
(https://github.com/bitcoin/bitcoin/pull/28162#issuecomment-1654522671)
Tested ACK 06199a995f20c55583f6948cfe99e608679fcdf1

Verified relevant tests still pass after cherry-picking 647d95a from #28166 to this branch.

I don't mind re-ACKing if you like the suggestion in https://github.com/bitcoin/bitcoin/pull/28162#discussion_r1276034368 (mind the spaces on the last line of its diff or run clang-format on it).
💬 brunoerg commented on pull request "fuzz: add target for `ScriptPubKeyMan` (legacy)":
(https://github.com/bitcoin/bitcoin/pull/28153#discussion_r1276789457)
Yea, just changed the approach to use buffer.
💬 brunoerg commented on pull request "fuzz: add target for `ScriptPubKeyMan` (legacy)":
(https://github.com/bitcoin/bitcoin/pull/28153#discussion_r1276790725)
Yes, just changed it.
💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1276818293)
I think it still makes sense because the functional test only covers the dns seed logic.
💬 mzumsande commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1276824800)
I think that usually it shouldn't substantially change the observations of a user, because 10 is a pretty high number and if all of those fail, we either have rally bad fixed seeds, or have a connectivity problem. I'll add a log entry with the next push though.
Also, I'll do some test runs to gather some statistics whether we might want to reduce this to 1 minute...
📝 hebasto opened a pull request: "ci: Run Windows native task on GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28173)
From https://github.com/bitcoin/bitcoin/issues/28098:
> Thus, someone would have to sponsor an amount of roughly 5kUSD/mo for those two tasks.

> If the goal is to stay on a free plan, I think the only option is GitHub Actions CI.

Historical context:
- https://github.com/bitcoin/bitcoin/pull/17697
- https://github.com/bitcoin/bitcoin/issues/17803
- https://github.com/bitcoin/bitcoin/pull/18031

Security concerns:
- https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-16514321
...
💬 luke-jr commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#discussion_r1276829144)
Prefer we just allow it for everything and error somewhere else
💬 luke-jr commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#discussion_r1276830223)
ipc*
🤔 furszy reviewed a pull request: "wallet: don't duplicate change output if already exist"
(https://github.com/bitcoin/bitcoin/pull/27601#pullrequestreview-1550813509)
> In general, we shouldn't assume what the change is when it is ambiguous - conceivably a user could be reusing addresses or otherwise actually wants a particular change address that is also one of the outputs. Doing this is allowed now, and I don't think we should change the meaning of the change address argument.

That is not really how the wallet behaves for regular outputs. Not sure why it should
behave differently for change outputs only.

Right now, the user cannot duplicate outputs i
...
📝 aureleoules opened a pull request: "docs: Rewrite README to make it more appealing"
(https://github.com/bitcoin/bitcoin/pull/28174)
The current README is pretty bland and I believe it could need a few improvements to enhance its value and appeal.

This pull request updates the README to make it more appealing. The changes are the following:
* Adding emojis 🚀 which makes the document more vibrant
* Use a more casual and interactive language to engage the reader

All the original technical contents were preserved, and the structure of the document remains the same to ensure that users still find the information they're
...
💬 luke-jr commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1654620797)
Approach NACK. If we're going to go this route, we can just add back the RPC method... :/
💬 luke-jr commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#issuecomment-1654628389)
Concept NACK, there's no reason such an address shouldn't be used.
💬 luke-jr commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1276860339)
I don't like outright lying in the UA. Why not just send it blank? Or at least some kind of standard for "I am not telling you"
🤔 jonatack reviewed a pull request: "docs: Rewrite README to make it more appealing"
(https://github.com/bitcoin/bitcoin/pull/28174#pullrequestreview-1550854157)
A few suggestions.

(I'm not sure who the emojis and "We" narration would be more appealing to and which target audience is being aimed for, but no strong opinion. Sugarcoating things might be a little akin to false advertising, though 😄.)
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276850613)
Use newlines consistently.
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276847563)
"evolving with regular builds" seems awkward and may not be interpreted the same as "regularly built"
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276850279)
```suggestion
We use the https://github.com/bitcoin-core/gui repository solely for GUI development. Its master branch serves as a clone in all monotree repositories. It doesn't have release branches and tags, so you only need to fork it for development purposes.
```

or s/We use/We use the/, s/tags here/tags there/ and s/fork this/fork that/
💬 jonatack commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1276855045)
I like "Please be patient and help out by reviewing and testing" better than "Please understand the delay and assist by testing". The bottleneck aspect might be helpful to keep.