💬 ajtowns commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601902513)
This is already thread safe due to its internal locks. If its only user is going to protect it anyway, the internal locking should go away, no?
> (not done in this PR) m_orphanage doesn't need its own lock anymore
Ah. Perhaps add `mutable Mutex m_mutex; // TODO: this lock is obsoleted by m_tx_download_mutex` in txorphanage.h
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601902513)
This is already thread safe due to its internal locks. If its only user is going to protect it anyway, the internal locking should go away, no?
> (not done in this PR) m_orphanage doesn't need its own lock anymore
Ah. Perhaps add `mutable Mutex m_mutex; // TODO: this lock is obsoleted by m_tx_download_mutex` in txorphanage.h
🤔 ajtowns reviewed a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2058441253)
I'm not sure how to be confident that the `UpdateBlockTipSync` / `hashRecentRejectsChainTip` would be correct and didn't miss an edge case. Rest looks good to me.
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2058441253)
I'm not sure how to be confident that the `UpdateBlockTipSync` / `hashRecentRejectsChainTip` would be correct and didn't miss an edge case. Rest looks good to me.
💬 ajtowns commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601944049)
Doesn't this also need to be called from `InvalidateBlock()`, potentially each time the tip is updated?
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601944049)
Doesn't this also need to be called from `InvalidateBlock()`, potentially each time the tip is updated?
💬 ajtowns commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601912655)
Do `AssertLockNotHeld` before the `AssertLockHeld` calls?
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601912655)
Do `AssertLockNotHeld` before the `AssertLockHeld` calls?
💬 hebasto commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2113040073)
This PR still have a couple of unaddressed comments: https://github.com/bitcoin-core/gui/pull/626#discussion_r915334998 and https://github.com/bitcoin-core/gui/pull/626#discussion_r1274077350.
@jadijadi Are you still working on this?
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2113040073)
This PR still have a couple of unaddressed comments: https://github.com/bitcoin-core/gui/pull/626#discussion_r915334998 and https://github.com/bitcoin-core/gui/pull/626#discussion_r1274077350.
@jadijadi Are you still working on this?
💬 brunoerg commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2113047528)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2113047528)
Concept ACK
💬 pinheadmz commented on issue "Apparently CJDNS network does not work with Tor on mainnet.":
(https://github.com/bitcoin/bitcoin/issues/24450#issuecomment-2113064951)
No PR yet but I am looking into which solution is best. Any thoughts ?
(https://github.com/bitcoin/bitcoin/issues/24450#issuecomment-2113064951)
No PR yet but I am looking into which solution is best. Any thoughts ?
💬 ismaelsadeeq commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#issuecomment-2113066401)
re-ACK 186a00e64490bb5b7b3c544346ae047ad1a66696
(https://github.com/bitcoin/bitcoin/pull/30072#issuecomment-2113066401)
re-ACK 186a00e64490bb5b7b3c544346ae047ad1a66696
💬 mozz30-tech commented on issue "Testsuite for Bitcoin Core 27.0.0 - FAIL: qt/test/test_bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2113082501)
> ### Is there an existing issue for this?
>
> - [X] I have searched the existing issues
>
> ### Current behaviour
>
> FAIL: qt/test/test_bitcoin-qt
> ============================================================================
> Testsuite summary for Bitcoin Core 27.0.0
> ============================================================================
> # TOTAL: 4
> # PASS: 3
> # SKIP: 0
> # XFAIL: 0
> # FAIL: 1
> # XPASS: 0
> # ERROR: 0
> =====================================================
...
(https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2113082501)
> ### Is there an existing issue for this?
>
> - [X] I have searched the existing issues
>
> ### Current behaviour
>
> FAIL: qt/test/test_bitcoin-qt
> ============================================================================
> Testsuite summary for Bitcoin Core 27.0.0
> ============================================================================
> # TOTAL: 4
> # PASS: 3
> # SKIP: 0
> # XFAIL: 0
> # FAIL: 1
> # XPASS: 0
> # ERROR: 0
> =====================================================
...
💬 mozz30-tech commented on issue "Restore wallet taking forever to load":
(https://github.com/bitcoin/bitcoin/issues/30108#issuecomment-2113082909)
> ### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
>
> - [X] I still think this issue should be opened here
>
> ### Report
>
> For some time now, I've been experiencing unusual slowdowns in Bitcoin Core, such as spending a long time without connecting to other pairs on the network and recently, I can't recover a wallet I want in the "restore wallet" option, it keeps loading for hours and never actually restores, this was never happening in L
...
(https://github.com/bitcoin/bitcoin/issues/30108#issuecomment-2113082909)
> ### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
>
> - [X] I still think this issue should be opened here
>
> ### Report
>
> For some time now, I've been experiencing unusual slowdowns in Bitcoin Core, such as spending a long time without connecting to other pairs on the network and recently, I can't recover a wallet I want in the "restore wallet" option, it keeps loading for hours and never actually restores, this was never happening in L
...
💬 mozz30-tech commented on issue "contrib/signet/miner: grind will fail for high difficulty chain":
(https://github.com/bitcoin/bitcoin/issues/30102#issuecomment-2113083239)
> ### Is there an existing issue for this?
>
> - [X] I have searched the existing issues
>
> ### Current behaviour
>
> Mining will fail with a `Could not satisfy difficulty target`.
>
> ```bash
> ❯ ~/2-development/bitcoin/bitcoin-core/contrib/signet/miner --cli "/Users/jose.edil/2-development/bitcoin/bitcoin-core/src/bitcoin-cli -datadir=/Users/jose.edil/2-development/bitcoin/signet-mining-experiments/signet-fix-hashing" generate --address tb1qylfujt900rjxzfxjj7sktpu7dpm2n9j60ch7jt --grind-c
...
(https://github.com/bitcoin/bitcoin/issues/30102#issuecomment-2113083239)
> ### Is there an existing issue for this?
>
> - [X] I have searched the existing issues
>
> ### Current behaviour
>
> Mining will fail with a `Could not satisfy difficulty target`.
>
> ```bash
> ❯ ~/2-development/bitcoin/bitcoin-core/contrib/signet/miner --cli "/Users/jose.edil/2-development/bitcoin/bitcoin-core/src/bitcoin-cli -datadir=/Users/jose.edil/2-development/bitcoin/signet-mining-experiments/signet-fix-hashing" generate --address tb1qylfujt900rjxzfxjj7sktpu7dpm2n9j60ch7jt --grind-c
...
💬 mozz30-tech commented on issue "decoderawtransaction should use hex or decimal, not both":
(https://github.com/bitcoin/bitcoin/issues/30067#issuecomment-2113083686)
> ### Is there an existing issue for this?
>
> - [X] I have searched the existing issues
>
> ### Current behaviour
>
> I have two different op_return outputs, but they are shown identically in their "asm" decoding:
>
> ```
> "scriptPubKey": {
> "asm": "OP_RETURN 1431655765",
> "hex": "6a051431655765",
> }
> ```
>
> ```
> "scriptPubKey": {
> "asm": "OP_RETURN 1431655765",
> "hex": "6a0455555555",
> }
> ```
>
> One shows the hex value.
...
(https://github.com/bitcoin/bitcoin/issues/30067#issuecomment-2113083686)
> ### Is there an existing issue for this?
>
> - [X] I have searched the existing issues
>
> ### Current behaviour
>
> I have two different op_return outputs, but they are shown identically in their "asm" decoding:
>
> ```
> "scriptPubKey": {
> "asm": "OP_RETURN 1431655765",
> "hex": "6a051431655765",
> }
> ```
>
> ```
> "scriptPubKey": {
> "asm": "OP_RETURN 1431655765",
> "hex": "6a0455555555",
> }
> ```
>
> One shows the hex value.
...
💬 mozz30-tech commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2113084941)
> The Bitcoin Core RPC interface [is implicitly versioned on the major version of Bitcoin Core](https://github.com/bitcoin/bitcoin/blob/42d5a1ff25a8045b6f4c09fa1fb71736dbccc034/doc/JSON-RPC-interface.md?plain=1#L67-L68). Some downstream RPC consumers and clients, for example rust-bitcoincore-rpc, need to know about the underlying node version to determine the available RPC calls and how to interpret the RPC responses (e.g. https://github.com/rust-bitcoin/rust-bitcoincore-rpc/issues/355).
>
> T
...
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2113084941)
> The Bitcoin Core RPC interface [is implicitly versioned on the major version of Bitcoin Core](https://github.com/bitcoin/bitcoin/blob/42d5a1ff25a8045b6f4c09fa1fb71736dbccc034/doc/JSON-RPC-interface.md?plain=1#L67-L68). Some downstream RPC consumers and clients, for example rust-bitcoincore-rpc, need to know about the underlying node version to determine the available RPC calls and how to interpret the RPC responses (e.g. https://github.com/rust-bitcoin/rust-bitcoincore-rpc/issues/355).
>
> T
...
👍 hebasto approved a pull request: "wallet: Allow user to navigate options while encrypting at creation"
(https://github.com/bitcoin-core/gui/pull/722#pullrequestreview-2058631259)
ACK cccddc03f0c625daeac7158eb20c1508aea5df39, tested on Ubuntu 24.04.
(https://github.com/bitcoin-core/gui/pull/722#pullrequestreview-2058631259)
ACK cccddc03f0c625daeac7158eb20c1508aea5df39, tested on Ubuntu 24.04.
✅ hebasto closed an issue: "Add go back button to `Confirm wallet encryption` window, add cancel button to `wallet to be encrypted` window"
(https://github.com/bitcoin-core/gui/issues/394)
(https://github.com/bitcoin-core/gui/issues/394)
🚀 hebasto merged a pull request: "wallet: Allow user to navigate options while encrypting at creation"
(https://github.com/bitcoin-core/gui/pull/722)
(https://github.com/bitcoin-core/gui/pull/722)
✅ maflcko closed an issue: "decoderawtransaction should use hex or decimal, not both"
(https://github.com/bitcoin/bitcoin/issues/30067)
(https://github.com/bitcoin/bitcoin/issues/30067)
💬 maflcko commented on issue "decoderawtransaction should use hex or decimal, not both":
(https://github.com/bitcoin/bitcoin/issues/30067#issuecomment-2113105730)
Closing for now as duplicate
(https://github.com/bitcoin/bitcoin/issues/30067#issuecomment-2113105730)
Closing for now as duplicate
💬 ryanofsky commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1602025691)
> > Flatten avoid an unnecessary move:
> What do you mean with this?
I was suggesting returning `Options&&` instead of `Options` to be consistent with the validation.cpp flatten code, and to avoid two moves potentially happening on return: One move on the `return opts;` statement which the compiler can't elide because `opts` is not a local variable, and another move after the function returns to initialize `m_opts` member.
In practice, however, it looks like only one move will happen, b
...
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1602025691)
> > Flatten avoid an unnecessary move:
> What do you mean with this?
I was suggesting returning `Options&&` instead of `Options` to be consistent with the validation.cpp flatten code, and to avoid two moves potentially happening on return: One move on the `return opts;` statement which the compiler can't elide because `opts` is not a local variable, and another move after the function returns to initialize `m_opts` member.
In practice, however, it looks like only one move will happen, b
...
👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2058653021)
Code review ACK 33f2a708e392edb2501f555efef34a558da9d717. I actually didn't notice this push when I made my last two comments, so feel free to ignore the comments if you want to keep the current code.
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2058653021)
Code review ACK 33f2a708e392edb2501f555efef34a558da9d717. I actually didn't notice this push when I made my last two comments, so feel free to ignore the comments if you want to keep the current code.