💬 ryanofsky commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1470140068)
> I'm not sure how to proceed on this, might need input from other reviewers. `"jsonrpc":"1.0"` is not a standard anywhere, not even in Bitcoin... but it has been in the RPC help text for a looooong time 😭

This is a good find, and I haven't looked closely, but it seems like there is not any conflict here. We can continue to accept `"jsonrpc":"1.0"` for compatibility, continue to require `"jsonrpc":"2.0" for JSON-RPC 2.0 behavior to comply with the 2.0 spec, and reject any other values so if
...

💬 josibake commented on pull request "net: enable v2transport by default":
(https://github.com/bitcoin/bitcoin/pull/29347#issuecomment-1915469470)
ACK https://github.com/bitcoin/bitcoin/pull/29347/commits/292e716cde3325bef83e78f7804d2d0bddf03509 :rocket:

👍 kristapsk approved a pull request: "net: enable v2transport by default"
(https://github.com/bitcoin/bitcoin/pull/29347#pullrequestreview-1849674537)
cr utACK 292e716cde3325bef83e78f7804d2d0bddf03509

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1470160697)
would still like a good explanation for this

💬 instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1470161011)
still waiting on some feedback on this point before moving forward with the PR

💬 instagibbs commented on pull request "net: enable v2transport by default":
(https://github.com/bitcoin/bitcoin/pull/29347#issuecomment-1915501310)
ACK https://github.com/bitcoin/bitcoin/pull/29347/commits/292e716cde3325bef83e78f7804d2d0bddf03509

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1470185755)
ok, carved out `"jsonrpc":"1.0"` and tested with push to ec5e7cea2bddfd55f07f9b0654f11d60f5ab0a48

💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1915548465)
I think the discussion on this PR is so spirited because it speaks to a more fundamental question of how to make changes to the bitcoin network. Please let me know if there is a more appropriate forum for this discussion, I'm happy to take this elsewhere.

> @chrisguida Let me expand on my NACK: given how easy it is for miners to adopt profit-maximizing transaction policies, and how easy it is for people to relay profit-maximizing transactions to them, we should not try to filter out profitabl
...

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-1915579486)
Thank you for your comments @josibake @glozow @achow101 , will address review comments shortly.

> if you typo'd a feerate orders of magnitude higher, your maxtxfee should get hit.

I don't think this is true; it depends on what your `maxtxfee` is. Going by the default value of 0.10 BTC.

This is not likely to occur.

I did a manual test on master.

When I set `maxtxfee` to 0.10 BTC on master, if you mistakenly type 10,000 as the fee rate, the max fee exceed error will not be hit.


...

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-1915582287)
> just seems really gross to have to now consider both of these in coin selection, especially if a user sets wonky values.

I dont really understand how this can affect coinselection, can you expand please? Thanks

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1469871777)
```suggestion
int64_t fee;
int32_t size;
```

Should their be more information that will describe the transaction size and fee, base / modified fee, vsize in bytes or something of that sort.
If not I think its okay like this.

---
Unrelated just asking to learn.
Why are'nt we using `CAmount` here for the fee?

In some places I see transaction size as `uint32_t` while some places its `int32_t`.

Should we have a type for size just like `CAmount`?

💬 dooglus commented on issue "New crash in v26.0":
(https://github.com/bitcoin-core/gui/issues/785#issuecomment-1915597249)
That didn't take too long:

```
[New Thread 0x7ffe7abff6c0 (LWP 419888)]
[New Thread 0x7ffe7a3fe6c0 (LWP 419889)]

Thread 1 "bitcoin-qt-v26." received signal SIGSEGV, Segmentation fault.
QSortFilterProxyModelPrivate::build_source_to_proxy_mapping (
proxy_to_source=..., source_to_proxy=..., this=this@entry=0x555557a8f3c0)
at ../../include/QtCore/../../src/corelib/tools/qarraydata.h:61
61 ../../include/QtCore/../../src/corelib/tools/qarraydata.h: No such file or directory.

...

💬 dooglus commented on issue "New crash in v26.0":
(https://github.com/bitcoin-core/gui/issues/785#issuecomment-1915600627)
I'll leave the gdb session running for a few days in case there's anything helpful I can type at it. Let me know.

🤔 murchandamus reviewed a pull request: "wallet: track mempool conflicts with wallet transactions"
(https://github.com/bitcoin/bitcoin/pull/27307#pullrequestreview-1849792773)
ACK 71059cf4267d80d2934b85a7046fc7b64900378b

💬 murchandamus commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1470235298)
In a6ae5b23b1497ab6f4899db49348db623700a2d8 "wallet: track mempool conflicts":

Nit: Should this perhaps also inform about the conflicting transactions like the _BlockConflicted_ state informs about the block?

💬 murchandamus commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1470229826)
I don’t feel strongly about it, please feel free to mark this comment as resolved.

💬 InsanityMatrix commented on issue "Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/issues/27494#issuecomment-1915645704)
Hey, as I haven't seen this being tackled I will happily undertake it. Currently working on it at https://github.com/InsanityMatrix/bitcoin on branch "signet-datadir-changes"

I plan to start with the src/init.cpp file and work my way through the files to change any references to the block directory depending on signet or mainnet, and also plan on making it backwards compatible. Any help would be appreciated, just mention me if you have any ideas or contributions! :)

🤔 pablomartin4btc reviewed a pull request: "net: enable v2transport by default"
(https://github.com/bitcoin/bitcoin/pull/29347#pullrequestreview-1849860045)
ACK 292e716cde3325bef83e78f7804d2d0bddf03509

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#issuecomment-1915658857)
Rebased to fix CI

💬 sr-gi commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#issuecomment-1915659529)
Rebased to fix CI