Bitcoin Core Github
44 subscribers
121K links
Download Telegram
:lock: fanquake locked an issue: "Make your trading journey smooth and easy."
(https://github.com/bitcoin/bitcoin/issues/28128)
💬 MarcoFalke commented on pull request "fuzz: use `ConnmanTestMsg` in `connman`":
(https://github.com/bitcoin/bitcoin/pull/28091#discussion_r1271303125)
Yeah, I am asking for the compiler warning when it is removed, or the method that requires it. Am I missing something?
💬 theStack commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1271314036)
Didn't verify my theory in practice yet, but according to my calculations having a HRP of size 4 would exceed the current 117 character limit (see https://github.com/bitcoin/bips/pull/1458/files#r1271312933). I assume the regtest-HRP should just also be set to "tsp" here (as it's currently stated in the BIP)? Otherweise the limit has to be raised by one.

Fun fact: in #27827 the "sprt" HRP works, but this is because the pubkeys are encoded as x-only (see commit https://github.com/bitcoin/bitco
...
💬 martinus commented on pull request "refactor: extract CCheckQueue's data handling into a separate container "Bag"":
(https://github.com/bitcoin/bitcoin/pull/27331#issuecomment-1646624160)
Thanks @jonatack , I've rebased 6b0537c -> 6a9c6ea. Fixed Makefile & include conflict, and added a `reserve()` in a unit test to fix a clang-tidy warning.
💬 brunoerg commented on pull request "fuzz: use `ConnmanTestMsg` in `connman`":
(https://github.com/bitcoin/bitcoin/pull/28091#discussion_r1271318560)
In fact, since we're using only `AddTestNode` I think that it's not required. Going to remove it..
💬 manfreddd commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646636639)
I was not sure how best to get the gettxoutsetinfo muhash output. So, I ran Bitcoin Core three different ways:

1. Run from the command line with coinstatsindex option (Bitcoincore.org doc recommends it).
[debug CLI coinstatsindex.log](https://github.com/bitcoin/bitcoin/files/12136921/debug.CLI.coinstatsindex.log)

3. Run from the command line with no options.
[debug CLI.log](https://github.com/bitcoin/bitcoin/files/12136923/debug.CLI.log)

4. Run from the graphical user interface and ge
...
💬 MarcoFalke commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646638775)
> Run from the graphical user interface and `gettxoutsetinfo muhash` from command window (crashed within 10 minutes).


It shouldn't crash, so there may be data corruption that leads leveldb into a crash.

I am not sure how to debug crashes on Windows, so someone else will need to help you there. (Maybe gdb exists?)


Otherwise, you can discard the leveldb chainstate by doing a full `-reindex-chainstate` (takes a long time).
💬 manfreddd commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646647499)
@MarcoFalke thank you for the support effort. Do I lose the Bitcoin data base loaded on my machine by discarding the leveldb chainstate?
💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1646648548)
Updated 67e172a7c0d82271a13f77f76ce72799faddd05c -> b89567f51ade926af8c918e4787046b7ccec8eb0 ([kernelRmUnivalue_3](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_3) -> [kernelRmUnivalue_4](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmUnivalue_3..kernelRmUnivalue_4))

* Fixed fuzz test by catching the UniValue exception.
🤔 hebasto reviewed a pull request: "kernel: Remove UniValue from kernel library"
(https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1542134637)
Approach ACK b89567f51ade926af8c918e4787046b7ccec8eb0. Nice!

@TheCharlatan

Do I understand you correctly that your intention is to make the `ParseSighash` quite generic?
💬 hebasto commented on pull request "Remove C-style const-violating cast, Use reinterpret_cast":
(https://github.com/bitcoin/bitcoin/pull/28127#issuecomment-1646655381)
> Using a C-style cast to convert pointer types to a byte-like pointer type has many issues

Can `clang-tidy` or other tools be helpful in catching such cases in the future?
💬 emc99 commented on pull request "Remove C-style const-violating cast, Use reinterpret_cast":
(https://github.com/bitcoin/bitcoin/pull/28127#issuecomment-1646657942)
> Using a C-style cast to convert pointer types to a byte-like pointer type has many issues:
>
> * It may accidentally and silently throw away `const`.
> * It forces reviewers to check that it doesn't accidentally throw away `const`.
>
> For example, on current master a `const char*` is cast to `unsigned char*` (without `const`), see
>
> https://github.com/bitcoin/bitcoin/blob/d23fda05842ba4539b225bbab01b94df0060f697/src/span.h#L273
>
> . This can lead to UB, and the only reason why
...
💬 sipa commented on pull request "Remove C-style const-violating cast, Use reinterpret_cast":
(https://github.com/bitcoin/bitcoin/pull/28127#issuecomment-1646658996)
UB = Undefined behavior
💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1646668269)
Re https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1542134637

> Do I understand you correctly that your intention is to make the ParseSighash quite generic?

Maybe, but I mostly thought it might be worthwhile to keep this utility around.
📝 petertodd opened a pull request: "Remove arbitrary restrictions on OP_RETURN by default"
(https://github.com/bitcoin/bitcoin/pull/28130)
Any number or size of data-carrying OP_RETURN outputs are allowed, and the `-datacarrier` option is removed. For those who want to limit data carrying outputs, `-datacarriersize` is still supported, and has the functionality of applying the specified data carrier limit as well as limiting the number of data carrying OP_RETURN outputs to one. If `-datacarriersize=0` is set, no data carrying output is allowed.

Rational: there's lots of ways for people to publish data in the Bitcoin chain, a
...
💬 petertodd commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1646718415)
Closing in favor of https://github.com/bitcoin/bitcoin/pull/28130
petertodd closed a pull request: "Ignore datacarrier limits for dataless OP_RETURN outputs"
(https://github.com/bitcoin/bitcoin/pull/27261)
💬 luke-jr commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646724534)
Concept NACK, the spam filters should be fixed, not removed or relaxed.
💬 jonatack commented on pull request "refactor: extract CCheckQueue's data handling into a separate container "Bag"":
(https://github.com/bitcoin/bitcoin/pull/27331#issuecomment-1646725108)
ACK 6a9c6ea9734854a2438d180ae13f123170e96d4c
🤔 tuanggo reviewed a pull request: "doc: Clarify -datacarriersize, add -datacarriersize=2 tests"
(https://github.com/bitcoin/bitcoin/pull/27832#pullrequestreview-1542179314)
open