💬 starius commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2571497025)
I attempted to test this on Signet with a 2-of-2 MuSig2 Taproot address (without script leaves).
Succeeded using `walletprocesspsbt`, but failed when using GUI "Load PSBT from keyboard" option.
**Setup:**
- **Node 1**: Watch-only, connected to the network.
- **Node 2**: Offline, holds the first private key.
- **Node 3**: Offline, holds the second private key.
**Steps to Reproduce:**
1. Imported descriptors for each node:
```
node 2 (first private key):
importdescrip
...
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2571497025)
I attempted to test this on Signet with a 2-of-2 MuSig2 Taproot address (without script leaves).
Succeeded using `walletprocesspsbt`, but failed when using GUI "Load PSBT from keyboard" option.
**Setup:**
- **Node 1**: Watch-only, connected to the network.
- **Node 2**: Offline, holds the first private key.
- **Node 3**: Offline, holds the second private key.
**Steps to Reproduce:**
1. Imported descriptors for each node:
```
node 2 (first private key):
importdescrip
...
💬 1440000bytes commented on issue "Fake bitcoin core website at the top of duckduckgo":
(https://github.com/bitcoin/bitcoin/issues/31602#issuecomment-2571545965)
> I haven't actually visited the site but I am assuming it is offering malware downloads to the unsuspecting. Is there anything that can be done about this?
Yes, downloads look malicious because I see different checksum for v27.0
You can report the search results: https://duckduckgo.com/duckduckgo-help-pages/company/contact-us
(https://github.com/bitcoin/bitcoin/issues/31602#issuecomment-2571545965)
> I haven't actually visited the site but I am assuming it is offering malware downloads to the unsuspecting. Is there anything that can be done about this?
Yes, downloads look malicious because I see different checksum for v27.0
You can report the search results: https://duckduckgo.com/duckduckgo-help-pages/company/contact-us
💬 TheCharlatan commented on pull request "rpc: fix mintime field testnet4, expand timewarp attack test":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2571595908)
> Additionally this PR adjusts the timewarp test to better illustrate the griefing attack discussed here
I think adding the example to the functional test is good, but it is not clear from the description what the griefing attack is that it seeks to prevent.
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2571595908)
> Additionally this PR adjusts the timewarp test to better illustrate the griefing attack discussed here
I think adding the example to the functional test is good, but it is not clear from the description what the griefing attack is that it seeks to prevent.
💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2571604696)
Putting in draft while I fix failing CI
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2571604696)
Putting in draft while I fix failing CI
📝 Eunovo converted_to_draft a pull request: "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict"
(https://github.com/bitcoin/bitcoin/pull/29680)
This PR implements a fix for the issue described in https://github.com/bitcoin/bitcoin/issues/29435.
The problem is that the wallet is unable to abandon transactions that have unrelated parent conflicts. The solution implemented here, augments the mempool transaction `REPLACED` signal with the double-spending transaction which the wallet stores and watches for in Block notifications. A map is added to the wallet to track conflicting tx ids and their child transactions. The entry is erased wh
...
(https://github.com/bitcoin/bitcoin/pull/29680)
This PR implements a fix for the issue described in https://github.com/bitcoin/bitcoin/issues/29435.
The problem is that the wallet is unable to abandon transactions that have unrelated parent conflicts. The solution implemented here, augments the mempool transaction `REPLACED` signal with the double-spending transaction which the wallet stores and watches for in Block notifications. A map is added to the wallet to track conflicting tx ids and their child transactions. The entry is erased wh
...
🤔 TheCharlatan reviewed a pull request: "mining: bugfix: Fix duplicate coinbase tx weight reservation"
(https://github.com/bitcoin/bitcoin/pull/31384#pullrequestreview-2530880952)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/31384#pullrequestreview-2530880952)
Approach ACK
💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903255292)
Typo nit: `s/for coinbase/for the coinbase/`
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903255292)
Typo nit: `s/for coinbase/for the coinbase/`
💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903257389)
Nit: I don't think adding these comments after an include is useful. They can become stale easily and it adds to the general inconsistencies of the codebase. Can you remove it again?
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903257389)
Nit: I don't think adding these comments after an include is useful. They can become stale easily and it adds to the general inconsistencies of the codebase. Can you remove it again?
💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903271412)
Is there a material difference between this test block and the previous one that actually exercises the default coinbase weight? Could more exact weight accounting where the total weight is checked with `assert_equal` instead of `assert_greater_than_or_equal` in `verify_block_template` be useful?
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903271412)
Is there a material difference between this test block and the previous one that actually exercises the default coinbase weight? Could more exact weight accounting where the total weight is checked with `assert_equal` instead of `assert_greater_than_or_equal` in `verify_block_template` be useful?
💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903265703)
Nit: Since you are creating a constant for the normal feerate, maybe create one for the fee rate of the large transactions too?
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903265703)
Nit: Since you are creating a constant for the normal feerate, maybe create one for the fee rate of the large transactions too?
💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903276616)
Does this have an effect on the log line in `miner.cpp::158`? Maybe clarify there that the numbers are given without the coinbase?
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903276616)
Does this have an effect on the log line in `miner.cpp::158`? Maybe clarify there that the numbers are given without the coinbase?
💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903268301)
Nit: This is a bit confusing, I think something like `(LARGE_TXS_COUNT - 1) + NORMAL_TXS_COUNT` would be more accurate, or just give the number like you do in the next test block.
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903268301)
Nit: This is a bit confusing, I think something like `(LARGE_TXS_COUNT - 1) + NORMAL_TXS_COUNT` would be more accurate, or just give the number like you do in the next test block.
💬 TheCharlatan commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903278355)
I'm a bit confused by this second sentence. How does the behaviour introduced here allow for overriding the default?
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903278355)
I'm a bit confused by this second sentence. How does the behaviour introduced here allow for overriding the default?
✅ fjahr closed a pull request: "sync: improve CCoinsViewCache ReallocateCache - 2nd try"
(https://github.com/bitcoin/bitcoin/pull/30370)
(https://github.com/bitcoin/bitcoin/pull/30370)
💬 fjahr commented on pull request "sync: improve CCoinsViewCache ReallocateCache - 2nd try":
(https://github.com/bitcoin/bitcoin/pull/30370#issuecomment-2571656801)
Alright, closing if the effect isn't significant. I have a few too many things on my plate at the moment anyway so maybe someone else wants to investigate further from here.
(https://github.com/bitcoin/bitcoin/pull/30370#issuecomment-2571656801)
Alright, closing if the effect isn't significant. I have a few too many things on my plate at the moment anyway so maybe someone else wants to investigate further from here.
📝 kehiy opened a pull request: "doc: upgrade Bitcoin Core license to 2025"
(https://github.com/bitcoin/bitcoin/pull/31605)
(https://github.com/bitcoin/bitcoin/pull/31605)
💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1903304101)
fixed
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1903304101)
fixed
💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1903304182)
`hasBlocks()` is already used one line above where I use `getPruneHeight()`. The problem is it doesn't tell us if we have been pruning or not.
Is there a concerns with exposing `GetPruneHeight()` explicitly? I think the function could be moved outside of RPC code, it just wasn't needed anywhere else so far.
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1903304182)
`hasBlocks()` is already used one line above where I use `getPruneHeight()`. The problem is it doesn't tell us if we have been pruning or not.
Is there a concerns with exposing `GetPruneHeight()` explicitly? I think the function could be moved outside of RPC code, it just wasn't needed anywhere else so far.
💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1903304370)
Yeah, this works, I replaced it. Thanks!
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1903304370)
Yeah, this works, I replaced it. Thanks!
💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2571684342)
Addressed feedback from @mzumsande
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2571684342)
Addressed feedback from @mzumsande