🤔 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

🤔 ishaanam reviewed a pull request: "test: Bump walletpassphrase timeout to avoid intermittent issue"
(https://github.com/bitcoin/bitcoin/pull/28089#pullrequestreview-1542183956)
ACK fa257a727c27a85cffaed94d1a241e631b2afda3

I would also ACK a PR that creates a wallet specific to this test as discussed above.

💬 recursive-rat4 commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646803412)
NACK until the data is discardable and the block size is increased.

💬 izlan90 commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646804102)
Bagai mana saya nak lakukan sekarang. Saya tak tahu lagi niDihantar daripada Huawei Mobile saya-------- Mesej Asal --------Subjek: Re: [bitcoin/bitcoin] Remove arbitrary restrictions on OP_RETURN by default (PR #28130)Daripada: Pavel Vasin Kepada: bitcoin/bitcoin SK: Subscribed
NACK until the data is discardable and the block size is increased.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@**
...

💬 izlan90 commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646804437)
What am I going to do now... I don't know how to do anything else here.. I haven't traded for a long time.. Can you help meDihantar daripada Huawei Mobile saya-------- Mesej Asal --------Subjek: Re: [bitcoin/bitcoin] Remove arbitrary restrictions on OP_RETURN by default (PR #28130)Daripada: Pavel Vasin Kepada: bitcoin/bitcoin SK: Subscribed
NACK until the data is discardable and the block size is increased.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving t
...

💬 russeree commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#issuecomment-1646814647)
tACK

I applied this PRs logic with the `current branches ``.oneline_description``` and was able to get the desired error message.


💬 Ayms commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646816395)
Concept ACK bitcoin surpasses eth in every domain except storage
Current storage methods need two txs and many deviant methods were used in the past to store things in bitcoin, as we saw recently also

💬 eval-exec commented on issue "Win64 CI failure in feature_versionbits_warning.py":
(https://github.com/bitcoin/bitcoin/issues/28115#issuecomment-1646817279)
> `AssertionError: [node 0] Node returned unexpected exit code (3) vs (0) when stopping`

Hello, I think a clue to solve this issue is to find out where the `exit code (3)` is returned? But I didn't find where the `exit code (3)` is returned in the source code.
What do you think?

💬 russeree commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271432003)
The above suggestion doesn't compile without syntax changes due to ```error: unterminated argument list invoking macro "STR_INTERNAL_BUG"``` This is because of a missing ```)``` to close off the arguments for STR_INTERNAL_BUG and then there is a loose comma after ```m_opts.oneline_description```. Once these are removed code does compile but the output is not as clean as the current commit due to an extra ```"``` on a new line after the name of the ```oneline_description```

The output of ```ST
...

💬 russeree commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271432874)
The above suggestion doesn't compile without syntax changes due to ```error: unterminated argument list invoking macro "STR_INTERNAL_BUG"``` This is because of a missing ```)``` to close off the arguments for STR_INTERNAL_BUG and then there is a loose comma after ```m_opts.oneline_description```. Once these are removed code does compile but the output is not as clean as the current commit due to an extra ```"``` on a new line after the name of the ```oneline_description```

The output of ```ST
...