Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 Mackain reopened a pull request: "doc: update IBD requirements in doc/README.md"
(https://github.com/bitcoin/bitcoin/pull/30992)
A small change to the first paragraph of the Setup part of the README that has been bugging me for a while.
The disk space required for the Bitcoin transactions can no longer be described as "a few" hundred gigabytes.
So I thought it was time it was changed to "several" instead.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
h
...
💬 Mackain commented on pull request "doc: update IBD requirements in doc/README.md":
(https://github.com/bitcoin/bitcoin/pull/30992#issuecomment-2389012540)
> > The commit message was created automatically when I clicked the "Commit Suggestion" button on your previous feedback. Fixing it now.
>
> @Mackain The commit message wasn't updated yet, could you update it and re-push the same commit (thanks!)
>
> Edit: if helpful, reckon you could use the same commit message as the PR title.

Ah, sorry. I now understand what you mean. I originally thought you wanted me to update the commit message of [3208df2](https://github.com/bitcoin/bitcoin/commi
...
⚠️ tdb3 opened an issue: "ci: macOS 14 CI failure `Invalid value detected for '-wallet' or '-nowallet'`"
(https://github.com/bitcoin/bitcoin/issues/31019)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Encountered the following failure in PR #30793 (https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2387502458) for `CI / macOS 14 native, arm64, no depends, sqlite only, gui (pull_request)`. The failure wasn't seen on the other CI jobs.

### Expected behaviour

No failure

### Steps to reproduce

CI run with #39793 commit e6853592361341c27103ed74b25470ac1e098d6d

### Relevant log
...
💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2389078228)
> Please report the CI failure.
Issue #31019 opened
💬 tdb3 commented on issue "ci: macOS 14 CI failure `Invalid value detected for '-wallet' or '-nowallet'`":
(https://github.com/bitcoin/bitcoin/issues/31019#issuecomment-2389081000)
At the time of the failure, the PR commits were on top of commit a74bdeea1b8e27b2335f0f7da78006e87ecfb235 (Sept 2nd).
I'm probably missing something, but it almost seemed like the CI job was executing code that was introduced in commit ee47ca29d6ef55650a0af63bca817c5d494f31ef (dated Aug 22nd, from PR #30684, but merged with commit fb52023ee69c346dd101770716b6d8c0525c38aa on Sept 9th).
💬 Sjors commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2389082792)
I also get the segfault when rebuilding (depends and bitcoin-qt) with 354f5d79270c0784a6ba59d61191ee7eaaa2a53d.

```
% lldb build-depends/src/qt/bitcoin-qt
(lldb) target create "build-depends/src/qt/bitcoin-qt"
Current executable set to '/Users/sjors/dev/bitcoin/build-depends/src/qt/bitcoin-qt' (x86_64).
(lldb) run
Process 7195 launched: '/Users/sjors/dev/bitcoin/build-depends/src/qt/bitcoin-qt' (x86_64)
2024-10-02 18:18:32.237316+0200 bitcoin-qt[7195:247862] SecTaskLoadEntitlements fai
...
💬 jonatack commented on issue "rfc: DATUM mining interface requirements":
(https://github.com/bitcoin/bitcoin/issues/31002#issuecomment-2389106217)
> So far it's proprietary and undocumented, but hopefully that changes.

Agree. I believe it may be MIT-licensed and public soon.
🤔 glozow reviewed a pull request: "rpc: add getorphantxs"
(https://github.com/bitcoin/bitcoin/pull/30793#pullrequestreview-2343446923)
ACK b6368fc285b. I have some picky review comments if you're interested, but I don't feel that strongly on anything except the default argument one.

I tested with @0xB10C's visualizer as well, very cool!
<img width="833" alt="image" src="https://github.com/user-attachments/assets/c1e81c89-3d4d-4a5a-a092-51d633ea10db">
💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1784873140)
It is a bit funky that a negative verbosity turns into 0! I was kind of expecting a `JSONRPCError`. I don't mind and it looks like it's the same for `getblock`, so not asking for a change, but could be worth a test case?
💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1784854966)
nit: comment is unnecessary. Also my personal preference is to not put them on the same line
💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1784861016)
an annotation can help here
```suggestion
int verbosity{ParseVerbosity(request.params[1], /*default_verbosity=*/1)};
```
💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1784859575)
I find default arguments to be pretty footgunny and prefer to require explicit. Since you're introducing this function for the first time, I suggest no default arg.

```suggestion
int ParseVerbosity(const UniValue& arg, int default_verbosity);
```
🤔 instagibbs reviewed a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2343039535)
some last nits/suggestions
💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1784580197)
not strictly a superset, right? Uses `NON_GOOD_RESPONSES` if not good which excludes `TX_REAL`
💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1784885638)
I'm not sure what this comment means. I know the `honestid` peer needs to be wtxid relaying to ensure that it can properly serve `malleated_tx` when necessary, otherwise `GetCorrectTx` will fetch the wrong response by giving back `correct_tx`.
💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1784688753)
This should catch issues marking things as received properly, at least for wtxid version since we're forcing wtxid relay for good peer.
```Suggestion
// Honest peer only tells us about INV_REAL in this scenario, should be empty since REAL_TX received
if (peer.m_nodeid == honestid) {
txdownloadman.CheckIsEmpty(honestid);
}
txdownloadman.DisconnectedPeer(peer.m_nodeid);
```
💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1784582239)
Could use `PickCoins` here as well
⚠️ Khalilheyrani6367 opened an issue: "Bitcoin 💵"
(https://github.com/bitcoin/bitcoin/issues/31020)
Bitcoin (BTC) Address: bc1qumf5ap6g85qanrrjhutqrkrvl3phch4fn66frn
💬 Khalilheyrani6367 commented on issue "Bitcoin 💵":
(https://github.com/bitcoin/bitcoin/issues/31020#issuecomment-2389135162)
Deposit bitcoin wallet exodus
pinheadmz closed an issue: "Bitcoin 💵"
(https://github.com/bitcoin/bitcoin/issues/31020)