💬 1440000bytes commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-2726286747)
Concept ACK

💬 ajtowns commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1996622142)
> I thought the only reason you had this logic here was to convey to the RPC whether to show these fields, which is a bit convoluted. I don't think this is worth touching at this point.

Ah, no, it's two separate things in my head: the versionbits code quoted above is making sure the values returned in the structure are as sane as they can be, and the RPC code is taking a special case that wouldn't make sense (threshold=0 always implies possible=true if the signalling means anything) and tryin
...

📝 maflcko opened a pull request: "contrib: Make deterministic-coverage error messages more readable"
(https://github.com/bitcoin/bitcoin/pull/32074)
This is almost a "refactor" to tidy up the error messages. Apart from the messages, the behavior of the tools is identical.

This was requested in https://github.com/bitcoin/bitcoin/pull/31901#discussion_r1969502508.

💬 maflcko commented on pull request "test: testnet4 could log a disk space warning":
(https://github.com/bitcoin/bitcoin/pull/32057#discussion_r1996663663)
not sure about logging a warning, then checking it and manually removing it, when the warning isn't even part of the test. It seems easier to just avoid the warning by setting `prune=whatever`?

I expect this will make the testnet3 removal diff a tiny bit smaller, as well as the testnet5 addition diff.

🤔 maflcko reviewed a pull request: "build: Drop option to disable hardening."
(https://github.com/bitcoin/bitcoin/pull/32071#pullrequestreview-2687522118)
review ACK ecf2046d4b5c43ddf64f62f09cd3ed70dd5caafb 📮

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK ecf2046d4b5c
...

💬 maflcko commented on pull request "test: Update coverage.cpp to drop linux restriction":
(https://github.com/bitcoin/bitcoin/pull/32059#issuecomment-2726365221)
review-only ACK ecdbe72916c1cabbf810e892d25fe6eed30b1306

💬 maflcko commented on pull request "contrib: Add deterministic-unittest-coverage":
(https://github.com/bitcoin/bitcoin/pull/31901#discussion_r1996678675)
:nail_care: Applied nail polish to the messages in https://github.com/bitcoin/bitcoin/pull/32074

🤔 janb84 reviewed a pull request: "test: Use rpc_deprecated only for testing deprecation"
(https://github.com/bitcoin/bitcoin/pull/31977#pullrequestreview-2687613840)
re ACK [2819c51](https://github.com/bitcoin/bitcoin/commit/2819c514825577dc8d73690882f25117d412ed9f)
Thank you for rewriting the comment - the intention is much easier to understand now (IMO)

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1996761714)
It shouldn't be, but annoyingly this is not trivial to change. Since the functions constructing the params in our code only return const types, we have to carry that const into our API. I think the only alternative is copying the params, which I will push shortly.

🤔 polespinasa reviewed a pull request: "test: Use rpc_deprecated only for testing deprecation"
(https://github.com/bitcoin/bitcoin/pull/31977#pullrequestreview-2687625876)
reACK 2819c514825577dc8d73690882f25117d412ed9f

⚠️ farrelA opened an issue: "SAI-15 and i can execution idea."
(https://github.com/bitcoin/bitcoin/issues/32075)
### Please describe the feature you'd like to see added.

// SAI-15 Key Generation and Encryption Framework
def generate_key(self):
return secrets.token_bytes(399)

def encrypt(self, plaintext, key):
if len(key) != 399:
raise ValueError("Key must be exactly 399 bytes")
ciphertext = bytearray()
for i, byte in enumerate(plaintext):
key_byte = key[i % 399]
ciphertext.append(byte ^ key_byte)
return bytes(ciphertext)

// Message Schedule and Compression for
...

✅ maflcko closed an issue: "SAI-15 and i can execution idea."
(https://github.com/bitcoin/bitcoin/issues/32075)

💬 maflcko commented on pull request "test: Use rpc_deprecated only for testing deprecation":
(https://github.com/bitcoin/bitcoin/pull/31977#issuecomment-2726465393)
lgtm ACK 2819c514825577dc8d73690882f25117d412ed9f

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1996815658)
> This (+ in log_category_to_string() leads to runtime assertion errors for interpreted languages

I think the problem here is that the C enums are weakly typed, and if you use them in weakly typed languages you run into these problems. I think the function signature should already give enough of a hint on what the range of allowed values is. That said, I'll change this to instead return `std::nullopt` ad then return false from there.

💬 maflcko commented on pull request "fuzz: provide more realistic values to the base58(check) decoders":
(https://github.com/bitcoin/bitcoin/pull/31917#issuecomment-2726501066)
review ACK d5537c18a9034647ba4c9ed4008abd7fee33989e 🚛

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK d5537c18a903
...

💬 maflcko commented on pull request "fuzz: provide more realistic values to the base58(check) decoders":
(https://github.com/bitcoin/bitcoin/pull/31917#issuecomment-2726515276)
(In theory this could be backported to 29.0 to avoid a fuzz timeout there, but it isn't too important, up to the backport maintainer)

⚠️ bearycool11 opened an issue: "Feature Request: Forking Bitcoin for Native gRPC/dRPC Integration"
(https://github.com/bitcoin/bitcoin/issues/32076)
### Please describe the feature you'd like to see added.

So I noticed on mintscan that we have bitcoin API, but... we don't have gRPC/dRPC hooking in bitcore, which is causing a lot of wacky stuff to occur on Coinbase.

Then again, if we're going to introduce this feature we to be careful of this following bug vulnerability

interface ProviderRpcError {
message: string;
code: number;
data?: { method?: string };
stack?: string;
}

Because we all know what that does. Coinbase, the cryp
...

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1996842801)
I don't think we need to rely on the order inside the enumeration here. That said, it could also just mirror the values in the `BCLog::Level`. I left out `Warning` and `Error` because you can't really control those right now.

💬 janb84 commented on issue "29.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/32026#issuecomment-2726538829)
> Thanks for working on this!
>
> > ... After running through the steps in this guide, you are encouraged to do your own testing.
> > This can be as simple as testing the same features in this guide but trying it a different way. ...
>
> These seem like they belong in the same paragraph? Either replace "This" with "Your own testing" or bring them together.

Changed ✅
>
> > introduces ephemeral dust to improve transaction packaging
>
> Ephemeral dust transactions should be combined with a tra
...

💬 bearycool11 commented on issue "Feature Request: Forking Bitcoin for Native gRPC/dRPC Integration":
(https://github.com/bitcoin/bitcoin/issues/32076#issuecomment-2726539486)
as for the coinbase SDK with the AI model causing further mayhem because I'm quite sure they didn't even look at my repos and just let the thing loose.... yeah that's another problem and bug we will need to resolvde-- I mean if you all just used Dr. Steve Scargal's and my adaptive persistent memory architecture, we wouldn't be having these issues with bugwatcher.c/.h and custodian.c/h,

Also the Github Staff needs to get off their butts and approve me so I can get sponsored and paid for this st
...