Bitcoin Core Github
43 subscribers
124K links
Download Telegram
🚀 fanquake merged a pull request: "[23.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/26921)
💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1433329273)
I pushed some changes (with @Xekyo's permission, thanks):
- Capped traversal at 500 items in `CalculateCluster()`. Number is arbitrary, open for commentary
- Fixed up a few things in the MiniMiner implementation, mostly shuffling things around and updating comments
- Dropped the chain interface changes (I think those can go in #26152)
- Expanded unit tests
- Added a fuzzer (expanded from @dergoegge's, thanks)
- Added a fuzzer to differentially test block templates built by `BlockAssembler`
...
💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-1433331637)
Would this change need a `release-notes-27064.md` file? It is kinda changing a configuration model
📝 TheCharlatan review_request_removed a pull request: "refactor / kernel: Move non-gArgs chainparams functionality to kernel"
(https://github.com/bitcoin/bitcoin/pull/26177)
This pull request is mostly a code-move of chainparams related functionality into the kernel library. @dongcarl is the original author of this patchset, these commits were taken from https://github.com/dongcarl/bitcoin/tree/2022-03-libbitcoinkernel-chainparams-args-only.

#### Context

The bitcoin kernel library currently relies on code containing user configurations through the `ArgsManager`. This is not optimal, since as a stand-alone library it should not rely on most of the user-define
...
📝 TheCharlatan review_request_removed a pull request: "refactor / kernel: Move non-gArgs chainparams functionality to kernel"
(https://github.com/bitcoin/bitcoin/pull/26177)
This pull request is mostly a code-move of chainparams related functionality into the kernel library. @dongcarl is the original author of this patchset, these commits were taken from https://github.com/dongcarl/bitcoin/tree/2022-03-libbitcoinkernel-chainparams-args-only.

#### Context

The bitcoin kernel library currently relies on code containing user configurations through the `ArgsManager`. This is not optimal, since as a stand-alone library it should not rely on most of the user-define
...
💬 MarcoFalke commented on pull request "lint: enable E722 do not use bare except":
(https://github.com/bitcoin/bitcoin/pull/25867#issuecomment-1433342403)
> bare excepts should be replaced with specific exceptions, not except Exception.

I wonder if this should be done.

At least it is unclear to me how to review this easily. And if it is hard to review and maintain, I wonder if it is worth it.
💬 hebasto commented on pull request "src/randomenv.cpp: fix build on uclibc":
(https://github.com/bitcoin/bitcoin/pull/20358#issuecomment-1433349312)
Hmm, this patch looks [buggy](https://github.com/bitcoin/bitcoin/pull/25302#issuecomment-1433291262)...
📝 MarcoFalke unlocked a pull request: "src/randomenv.cpp: fix build on uclibc"
(https://github.com/bitcoin/bitcoin/pull/20358)
Check for HAVE_STRONG_GETAUXVAL or HAVE_WEAK_GETAUXVAL before using
getauxval to avoid a build failure on uclibc

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
💬 achow101 commented on pull request "refactor: Move coin_control variable to test setup section":
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1433353547)
> Tiny PRs like this in my mind should be reviewed and either closed or merged in a short period since it's trivial to review. I'm surprised it's taken 6 months to even be looked at.

The fact that a tiny PR like this one has taken 6 months for anyone to even look at it is why I think it is noise. There are over 300 other PRs to look at, with several large projects going on at the same time. People are busy looking at other things, and simply having this PR even exist means that time that coul
...
💬 MarcoFalke commented on pull request "refactor: Move coin_control variable to test setup section":
(https://github.com/bitcoin/bitcoin/pull/26154#issuecomment-1433359488)
I also have a template response:


Thank you for your contribution. While this stylistic change makes sense on its own, it comes at a cost and risk for the project as a whole. The weak motivation for the change does not justify the burden that it places on the project. A burden could be any of the following:

* Time spent on review
* Accidental introduction of bugs
* (Silent) merge conflicts, either in the branch or a backport branch. Those conflicts demand further developer and reviewer
...
⚠️ MarcoFalke opened an issue: "Intermittent issue in p2p_ibd_stalling.py self.wait_until(lambda: self.total_bytes_recv_for_blocks() == 172761)"
(https://github.com/bitcoin/bitcoin/issues/27112)
```
wget https://drahtbot.space/temp_scratch/p2p_ibd_stalling_96.tar.xz
tar -xvf p2p_ibd_stalling_96.tar.xz
test/functional/combine_logs.py -c ./p2p_ibd_stalling_96
```

```
test 2023-02-14T20:54:45.479000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/root/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 134, in
...
📝 achow101 opened a pull request: "rpc: Use a FlatSigningProvider in decodescript to allow inferring descriptors for scripts larger than 520 bytes"
(https://github.com/bitcoin/bitcoin/pull/27113)
`FillableSigningProvider` limits scripts to 520 bytes even though segwit allows scripts to be larger than that. We can avoid this limit by using a `FlatSigningProvider` so that such larger scripts can be decoded.

Fixes #27111
💬 achow101 commented on issue "decodescript miniscript functionality tops out at 520 bytes":
(https://github.com/bitcoin/bitcoin/issues/27111#issuecomment-1433385518)
Fixed in #27113

A related question is whether `decodescript` should check script sizes? Other than this particular issue, I don't think it does any size checking.
💬 sipa commented on pull request "rpc: Use a FlatSigningProvider in decodescript to allow inferring descriptors for scripts larger than 520 bytes":
(https://github.com/bitcoin/bitcoin/pull/27113#issuecomment-1433385742)
Concept ACK
💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108701947)
```suggestion
std::vector<std::string> vec{SplitString(strCommand, " (),\n")};
auto should_remove{[](const std::string& str) { return str.empty(); }};
```
💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108713424)
```suggestion
auto executableCommand = "generatetoaddress " + nblocks + " " + address + " " + maxtries;
if (!RPCConsole::RPCExecuteCommandLine(m_node, result, executableCommand + "\n", nullptr, wallet_model)) {
```
Also remove the declaration of `executableCommand` near the top of the function; better to declare variables only where and when needed. You could do similarly with `result` and `address`.
💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108706223)
```suggestion
if (!RPCConsole::RPCExecuteCommandLine(m_node, address, "getnewaddress\n", /*pstrFilteredOut=*/nullptr, wallet_model)) {
```
💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108736164)
Formatting-only suggestion (personal preference to avoid long lines), and the argument to `QString()` had extra parentheses. Could make a similar change below.
```suggestion
if ((parsedCommand.size() > 1 && parsedCommand[0] == "help" && parsedCommand[1] == "generate") ||
(parsedCommand.size() > 3 && parsedCommand[0] == "generate")) {
auto message{
"\n"
"Generate blocks, equivalent to RPC getnewaddress followed by R
...
💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1108718036)
This line is indented one tab stop too much, and same for the two lines just below (after the `else`).
💬 instagibbs commented on pull request "rpc: Use a FlatSigningProvider in decodescript to allow inferring descriptors for scripts larger than 520 bytes":
(https://github.com/bitcoin/bitcoin/pull/27113#issuecomment-1433398818)
ACK https://github.com/bitcoin/bitcoin/pull/27113/commits/73ec4b2a8347c796b9aadc1f2576b286c469f9e7

thanks for the quick turnaround