Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1726654514)
bb3981c01985989b42da0a24c027be757ab54612: -> "are marked invalid"
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1726662966)
bb3981c01985989b42da0a24c027be757ab54612 `\"latest\"`?

The first time I read "latest type" as meaning a new type.
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1726770642)
b23abfca68f7b349e6d2f9e04369f0fdc82d2a98: a followup could handle multiple blocks at the same height by running a loop that invalidates all (new) tips at the target height that are not the block hash we expected.

Those could be tracked in `invalidate_indexes` and be reconsidered in reverse order (and with the network still off) after the snapshot is made.
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1726791942)
It seems conceptually simpler to first write the snapshot, reconsider all blocks that were marked invalid and only then restore network activity.

> make the node more usable while the snapshot is being created

Lower usability is probably feature. The user should not be doing anything important until the chain is back at the original tip, e.g. loading a wallet could still cause confusion.
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1726781190)
b23abfca68f7b349e6d2f9e04369f0fdc82d2a98: assuming this is blocking and it could take hours before the throw below, it would be good to emit a log message in the mean time.
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1726741569)
b23abfca68f7b349e6d2f9e04369f0fdc82d2a98: I had to read this description several times to get it, but the examples make it clear.

Having a third positional optional argument `height` would have seemed easier to me, but it's fine like this.
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1726658380)
bb3981c01985989b42da0a24c027be757ab54612: may be add: "This inconsistent state is also why network activity is temporarily disabled, causing us to disconnect from all peers."
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1726784278)
b23abfca68f7b349e6d2f9e04369f0fdc82d2a98: for a followup, please check if this path is writable early in the RPC function. I once wasted a few hours because I used `~/someplace` which it didn't understand.
💬 cryptoquick commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2304381245)
Apologies, I meant to say internal SATA drives. There's no USB enclosure, they're inside the case and plugged into SATA 3.

I tried upgrading the BIOS. I'll see if that helps.
💬 fanquake commented on pull request "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled":
(https://github.com/bitcoin/bitcoin/pull/30685#issuecomment-2304400723)
Guix building 89f8e8e7f5828b1d06925e036c16eda050b05c81 on `aarch64` doesn't work:
```bash
time HOSTS="x86_64-linux-gnu" ./contrib/guix/guix-build
<snip>
CXXLD test/test_bitcoin
x86_64-linux-gnu-ld: /bitcoin/depends/x86_64-linux-gnu/lib/libevent.a(buffer.c.o): error: missing IBT and SHSTK properties
x86_64-linux-gnu-ld: /bitcoin/depends/x86_64-linux-gnu/lib/libevent.a(bufferevent.c.o): error: missing IBT and SHSTK properties
<snip>
x86_64-linux-gnu-ld: /bitcoin/depends/x86_64-linux-g
...
🤔 glozow reviewed a pull request: "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test"
(https://github.com/bitcoin/bitcoin/pull/30681#pullrequestreview-2254182247)
ACK 59ff17e5af4e382cbe16f183767beef1bdcd9131
💬 glozow commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#discussion_r1726826469)
typo nit e85f386c4b157b7d1ac16aface9bd2c614e62b46
```suggestion
* Enforce BIP94 timewarp attack mitigation. On testnet4 this also enforces
```
💬 hebasto commented on issue "Control-flow application capabilities for `x86_64-linux-gnu` release binaries":
(https://github.com/bitcoin/bitcoin/issues/30677#issuecomment-2304408493)
> Does the ELF flag control whether the feature will be enabled at runtime?

I don't think so. From https://docs.kernel.org/next/arch/x86/shstk.html:
> The kernel does not process these applications markers directly.

However, the linker will not place these markers if all object files are not properly instrumented. The `cet-report=error` [linker flag](https://sourceware.org/binutils/docs/ld.html) can make these checks visible.
💬 hebasto commented on pull request "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled":
(https://github.com/bitcoin/bitcoin/pull/30685#discussion_r1726867429)
This is done to prevent disruption of internal flag checks.
💬 fanquake commented on pull request "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled":
(https://github.com/bitcoin/bitcoin/pull/30685#discussion_r1726869045)
What disruption?
💬 l0rinc commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726874669)
Thanks for considering
💬 l0rinc commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726876602)
Haven't realized it was possible before as well, thanks for clearing that up
💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1726878944)
The fuzz target is always setting `state.Invalid`, so I don't see where this coverage would come from?
💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1726881353)
hmm, must have been referencing something else, nevermind
💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726879771)
Resolved by removing `TrySanitizeHexNumber()` in latest force-push.