💬 hebasto commented on pull request "Fix transaction view/table":
(https://github.com/bitcoin-core/gui/pull/662#issuecomment-1570569786)
Closing due to a long period of inactivity here. Feel free to reopen.

✅ hebasto closed a pull request: "Fix transaction view/table"
(https://github.com/bitcoin-core/gui/pull/662)

💬 theuni commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1570613820)
> > just need to decide out what to do about guix.
>
> What options are currently being considered. Is an upstream patch still possible?

Yes, I'm still working on testing this. It changes how the llvm build works and I'd like to be more confident in the change before proposing it.

💬 denavila commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1570613994)
> Bitcoin Core's wallet has a "unique" fingerprint on transactions that is shared by way less than 50% of transactions in the network ...

By fingerprint, do you mean Bitcoin Core's transactions can be identified on the block chain? If that's the case that's unfortunate for sure. What's the nature of the difference? Is it on purpose and if not, would it be possible to fix?

💬 ryanofsky commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1570638653)
> Could we just have the server automatically convert the arguments as needed? This can still be done before the RPC execution since we have type information in the RPCHelpMan.

Oh you're right. That seems like a better idea, simpler than either of my suggestions. And I think it could allow getting rid of the `vRPCConvertParams` table entirely?

I think the only possible downside of having the server automatically convert unexpected string parameters to JSON numbers/bools/objects would be if
...

💬 achow101 commented on pull request "index: prevent race by calling 'CustomInit' prior setting 'synced' flag":
(https://github.com/bitcoin/bitcoin/pull/27720#issuecomment-1570652628)
ACK 3126454dcfa1dd29bb66500d5f2b5261684d6c58

💬 achow101 commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1570654857)
@ryanofsky Are you planning on making any further changes here?

💬 sr-gi commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1570662645)
So just to be clear, my concern here relates to the fact that the current codebase only allows a peer to request a transaction as long as we have announced that to them (with the exception of data requested after a `MEMPOOL` message if we happen run signaling bip37).

After this patch, the logic above won't hold true anymore. If for whatever reason we have more than `INV_BROADCAST_MAX` transactions to send to a given peer at a given time, and we send an INV message to it, it will immediately b
...

💬 Crypt-iQ commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1570668159)
I get this when compiling with both gcc and clang-16. I am also using libevent 2.1.12. Is there anything unique about your setup @willcl-ark ? Also, did you Ctrl-C the `waitforblockheight` command before trying to stop bitcoind?

🚀 achow101 merged a pull request: "index: prevent race by calling 'CustomInit' prior setting 'synced' flag"
(https://github.com/bitcoin/bitcoin/pull/27720)

💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212094169)
8990668: it would be useful to have radd and rsub back for [these operations in `xswiftec_inv`](https://github.com/bitcoin/bips/blob/master/bip-0324/reference.py#L367-L370).
(to keep #24005 consistent with BIP's reference python implementation for easy review.)

💬 sipa commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212129551)
I mean that the computation won't be repeated if called a second time (due to self.den being set to 1, which triggers the fast path). Whether that's accomplished through actually remembering the int-converted form or some other mechanism is an implementation detail a user of the class shouldn't need to care about.

💬 MarcoFalke commented on pull request "Add public Boost headers explicitly":
(https://github.com/bitcoin/bitcoin/pull/27783#issuecomment-1570717706)
lgtm ACK 2484cacb7a6367b24e924dba0825c843b1dfc1c3

🤔 Sjors reviewed a pull request: "Draft: rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746#pullrequestreview-1453921658)
If you can split 534b6f99a3779465678f39ed2704da6049c6469d in ~half that would make it a bit easier to review. Maybe start with anything that's trivial to move, and then do the more involved changes in the second commit.

💬 Sjors commented on pull request "Draft: rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1212154654)
534b6f99a3779465678f39ed2704da6049c6469d: this function seems to come out of nowhere and is only used in a test.

💬 ryanofsky commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1570744518)
> @ryanofsky Are you planning on making any further changes here?

I'm not working on anything. The only suggestion I saw was to rename the `fr` variable, but that is a preexisting variable so would prefer not to change that here.

💬 sipa commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212163089)
I guess we should; the parentheses are for listing parent classes, but I believe that an empty list or no parentheses at all don't make a difference.

Fixed.

💬 sipa commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212164841)
I've added a `"no overflow allowed"` to the comment.

💬 sipa commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212166978)
I've kept the `FE.is_square()` function, but replaced it with just a `x.sqrt() is not None` plus a comment that a more efficient algorithm is possible.

This leaves the option of easily adding a more efficient implementation back later if really worth it.

💬 sipa commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212163658)
Ok, I added back.