💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436629)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436629)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436652)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436652)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436670)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436670)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436690)
Indeed, fixed.
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436690)
Indeed, fixed.
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436779)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436779)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436813)
Moved to the right commit.
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435436813)
Moved to the right commit.
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435437218)
This function was intended to only return a `CExtPubKey` if all active descriptors shared the same xpub, so if they differed, it would return `std::nullopt`. The comment was stating what it was attempting to do, not what it was expecting.
In any case, having the caller determine what to do if there is more than one xpub in the active descriptors is probably better, so I've changed it to do that and this comment is no longer relevant.
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435437218)
This function was intended to only return a `CExtPubKey` if all active descriptors shared the same xpub, so if they differed, it would return `std::nullopt`. The comment was stating what it was attempting to do, not what it was expecting.
In any case, having the caller determine what to do if there is more than one xpub in the active descriptors is probably better, so I've changed it to do that and this comment is no longer relevant.
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435437249)
Done as suggested.
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435437249)
Done as suggested.
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435439477)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435439477)
Done
⚠️ derekm opened an issue: "Spam Filter Code in Tx Relay & Mempool Acceptance Ignores OFAC SDN Sanctions"
(https://github.com/bitcoin/bitcoin/issues/29137)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
As an American Bitcoin miner,
New transactions' inputs and outputs are not checked for OFAC SDN Sanctions against XBT,
Therefore I am exposed to the risk of imprisonment should I mine a block containing txns for specially designated nationals and blocked persons.
### Expected behaviour
As an American Bitcoin miner,
I should be able to configure my node to check incoming transactions f
...
(https://github.com/bitcoin/bitcoin/issues/29137)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
As an American Bitcoin miner,
New transactions' inputs and outputs are not checked for OFAC SDN Sanctions against XBT,
Therefore I am exposed to the risk of imprisonment should I mine a block containing txns for specially designated nationals and blocked persons.
### Expected behaviour
As an American Bitcoin miner,
I should be able to configure my node to check incoming transactions f
...
💬 derekm commented on issue "Spam Filter Code in Tx Relay & Mempool Acceptance Ignores OFAC SDN Sanctions":
(https://github.com/bitcoin/bitcoin/issues/29137#issuecomment-1868196209)
What XBT sanctions all American citizens must implement can be found here: https://www.treasury.gov/ofac/downloads/sanctions/1.0/sdn_advanced.xml
(https://github.com/bitcoin/bitcoin/issues/29137#issuecomment-1868196209)
What XBT sanctions all American citizens must implement can be found here: https://www.treasury.gov/ofac/downloads/sanctions/1.0/sdn_advanced.xml
💬 ryanofsky commented on pull request "wallet: reenable sethdseed for descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/29054#issuecomment-1868202512)
This is great, amazed you could implement this so quickly!
I was thinking about the void(KEY) idea more today, and think it would be good to make 3 tweaks:
1 - rename "void(KEY)" to "unused(KEY)"
2 - add logic to `importdescriptors` to delete any unused(KEY) descriptor as soon as the KEY which it references is used by an imported descriptor (taking into account both public and private parts of the key)
3 - add logic to `importdescriptors` to disallow importing an unused(KEY) if KEY is us
...
(https://github.com/bitcoin/bitcoin/pull/29054#issuecomment-1868202512)
This is great, amazed you could implement this so quickly!
I was thinking about the void(KEY) idea more today, and think it would be good to make 3 tweaks:
1 - rename "void(KEY)" to "unused(KEY)"
2 - add logic to `importdescriptors` to delete any unused(KEY) descriptor as soon as the KEY which it references is used by an imported descriptor (taking into account both public and private parts of the key)
3 - add logic to `importdescriptors` to disallow importing an unused(KEY) if KEY is us
...
💬 ryanofsky commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `void(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-1868202696)
This is great, amazed you could implement this so quickly!
I was thinking about the void(KEY) idea more today, and think it would be good to make 3 tweaks:
1 - rename "void(KEY)" to "unused(KEY)"
2 - add logic to `importdescriptors` to delete any unused(KEY) descriptor as soon as the KEY which it references is used by an imported descriptor (taking into account both public and private parts of the key)
3 - add logic to `importdescriptors` to disallow importing an unused(KEY) if KEY is us
...
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-1868202696)
This is great, amazed you could implement this so quickly!
I was thinking about the void(KEY) idea more today, and think it would be good to make 3 tweaks:
1 - rename "void(KEY)" to "unused(KEY)"
2 - add logic to `importdescriptors` to delete any unused(KEY) descriptor as soon as the KEY which it references is used by an imported descriptor (taking into account both public and private parts of the key)
3 - add logic to `importdescriptors` to disallow importing an unused(KEY) if KEY is us
...
💬 tshev commented on issue "Spam Filter Code in Tx Relay & Mempool Acceptance Ignores OFAC SDN Sanctions":
(https://github.com/bitcoin/bitcoin/issues/29137#issuecomment-1868221065)
Thank you for highlighting the issue!
(https://github.com/bitcoin/bitcoin/issues/29137#issuecomment-1868221065)
Thank you for highlighting the issue!
⚠️ YezikTech opened an issue: "Bitcoin: Operation Sweep - an L1 upgrade proposition"
(https://github.com/bitcoin/bitcoin/issues/29138)
The Bitcoin blockchain is designed primarily for storing transaction data, though it has also accommodated other types of data, including non-transaction elements like images (JPGs) and malicious uploads. While these elements may not directly impact the functionality of the blockchain, they pose security risks and clutter the network. To address these concerns, detection and mitigation of non-transaction and malicious elements in the Bitcoin blockchain is essential and tabled for incentivised ac
...
(https://github.com/bitcoin/bitcoin/issues/29138)
The Bitcoin blockchain is designed primarily for storing transaction data, though it has also accommodated other types of data, including non-transaction elements like images (JPGs) and malicious uploads. While these elements may not directly impact the functionality of the blockchain, they pose security risks and clutter the network. To address these concerns, detection and mitigation of non-transaction and malicious elements in the Bitcoin blockchain is essential and tabled for incentivised ac
...
✅ fanquake closed an issue: "Spam Filter Code in Tx Relay & Mempool Acceptance Ignores OFAC SDN Sanctions"
(https://github.com/bitcoin/bitcoin/issues/29137)
(https://github.com/bitcoin/bitcoin/issues/29137)
✅ fanquake closed an issue: "Bitcoin: Operation Sweep - an L1 upgrade proposition"
(https://github.com/bitcoin/bitcoin/issues/29138)
(https://github.com/bitcoin/bitcoin/issues/29138)
💬 theStack commented on pull request "refactor: share and use `GenerateRandomKey` helper":
(https://github.com/bitcoin/bitcoin/pull/28455#discussion_r1435589223)
Good catch, done.
(https://github.com/bitcoin/bitcoin/pull/28455#discussion_r1435589223)
Good catch, done.
💬 theStack commented on pull request "refactor: share and use `GenerateRandomKey` helper":
(https://github.com/bitcoin/bitcoin/pull/28455#issuecomment-1868285388)
Thanks for the reviews! I've tackled the [missing header include](https://github.com/bitcoin/bitcoin/pull/28455#discussion_r1434172881) and the two more instances where the refactoring can be done ([script_standard_Solver_failure](https://github.com/bitcoin/bitcoin/blob/7524fcff8625f0197be6cf84df285c39fcd5d6b6/src/test/script_standard_tests.cpp#L141) and [script_standard_ExtractDestination](https://github.com/bitcoin/bitcoin/blob/7524fcff8625f0197be6cf84df285c39fcd5d6b6/src/test/script_standard
...
(https://github.com/bitcoin/bitcoin/pull/28455#issuecomment-1868285388)
Thanks for the reviews! I've tackled the [missing header include](https://github.com/bitcoin/bitcoin/pull/28455#discussion_r1434172881) and the two more instances where the refactoring can be done ([script_standard_Solver_failure](https://github.com/bitcoin/bitcoin/blob/7524fcff8625f0197be6cf84df285c39fcd5d6b6/src/test/script_standard_tests.cpp#L141) and [script_standard_ExtractDestination](https://github.com/bitcoin/bitcoin/blob/7524fcff8625f0197be6cf84df285c39fcd5d6b6/src/test/script_standard
...
💬 ryanofsky commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435607572)
Since this PR is adding the `createwalletdescriptor` method, maybe this is a good place to list some ways it could be extended in the future:
- Probably it would be good not to require wallet to be unlocked when dealing with public keys. Currently the specified `hdkey` is unencrypted and reencrypted, but this shouldn't be necessary because the key is already in the wallet. (The only reason this seems to happen now is to copy the key, because internally we store keys in a slightly denormalized
...
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435607572)
Since this PR is adding the `createwalletdescriptor` method, maybe this is a good place to list some ways it could be extended in the future:
- Probably it would be good not to require wallet to be unlocked when dealing with public keys. Currently the specified `hdkey` is unencrypted and reencrypted, but this shouldn't be necessary because the key is already in the wallet. (The only reason this seems to happen now is to copy the key, because internally we store keys in a slightly denormalized
...