💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237500416)
I've corrected the code and will add a test case for this
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237500416)
I've corrected the code and will add a test case for this
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237500806)
Done
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237500806)
Done
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237501117)
Done
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237501117)
Done
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237501642)
I've moved the change to the correct commit
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237501642)
I've moved the change to the correct commit
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237502419)
I've removed it, pretty sure it was from a rebase
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237502419)
I've removed it, pretty sure it was from a rebase
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237502775)
Done
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237502775)
Done
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237503063)
Done
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237503063)
Done
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237503366)
Done
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2237503366)
Done
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#issuecomment-3128453950)
Thanks for the feedback @glozow and @achow101! I'm converting this to a draft while I work on some additional test cases.
(https://github.com/bitcoin/bitcoin/pull/32896#issuecomment-3128453950)
Thanks for the feedback @glozow and @achow101! I'm converting this to a draft while I work on some additional test cases.
📝 ishaanam converted_to_draft a pull request: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896)
This PR Implements the following:
- If creating a v3 transaction, `AvailableCoins` doesn't return unconfirmed v2 utxos (and vice versa)
- `AvailableCoins` doesn't return an unconfirmed v3 utxo if its transaction already has a child
- If a v3 transaction is kicked out of the mempool by a sibling, mark the sibling as a mempool conflict
- Throw an error if pre-selected inputs are of the wrong transaction version
- Allow setting version to 3 manually in `createrawtransaction` (uses commits from
...
(https://github.com/bitcoin/bitcoin/pull/32896)
This PR Implements the following:
- If creating a v3 transaction, `AvailableCoins` doesn't return unconfirmed v2 utxos (and vice versa)
- `AvailableCoins` doesn't return an unconfirmed v3 utxo if its transaction already has a child
- If a v3 transaction is kicked out of the mempool by a sibling, mark the sibling as a mempool conflict
- Throw an error if pre-selected inputs are of the wrong transaction version
- Allow setting version to 3 manually in `createrawtransaction` (uses commits from
...
💬 josibake commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2237520358)
Per the BIP, it does specify `coin_type` (which is defined in BIP44). BIP44 does reserve `1` as a registered coin type for Bitcoin test networks (i.e., testnet, signet). Revisiting the language in the BIP, I'm realising this is not worded strongly enough and should be updated.
I do think we should follow the convention of deriving mainnet keys and test keys from distinct chains in our tests. This also matches what we do in other functional tests, e.g., `wallet_descriptor.py`.
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2237520358)
Per the BIP, it does specify `coin_type` (which is defined in BIP44). BIP44 does reserve `1` as a registered coin type for Bitcoin test networks (i.e., testnet, signet). Revisiting the language in the BIP, I'm realising this is not worded strongly enough and should be updated.
I do think we should follow the convention of deriving mainnet keys and test keys from distinct chains in our tests. This also matches what we do in other functional tests, e.g., `wallet_descriptor.py`.
💬 Kingsleyioryoo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3128469585)
Good
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3128469585)
Good
💬 Sjors commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2237526035)
That's definitely a good idea. Some hardware wallets may even refuse to sign if the coin type derivation is wrong, e.g. to prevent a testnet app from moving mainnet coins.
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2237526035)
That's definitely a good idea. Some hardware wallets may even refuse to sign if the coin type derivation is wrong, e.g. to prevent a testnet app from moving mainnet coins.
💬 achow101 commented on pull request "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver":
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3128482538)
> At this point, it's unclear whether incorporating these OpenSSL-based cryptographic functions directly into the Bitcoin Core codebase is practical or desirable.
I don't think it is desirable. OpenSSL is a massive codebase, and we only need a tiny part. We spent a very long time trying to remove OpenSSL as well.
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3128482538)
> At this point, it's unclear whether incorporating these OpenSSL-based cryptographic functions directly into the Bitcoin Core codebase is practical or desirable.
I don't think it is desirable. OpenSSL is a massive codebase, and we only need a tiny part. We spent a very long time trying to remove OpenSSL as well.
📝 pablomartin4btc opened a pull request: "wallet, refactor: Remove Legacy check and error"
(https://github.com/bitcoin/bitcoin/pull/33082)
Remove dead code due to legacy wallet removal.
Leftovers from previous #32481.
(https://github.com/bitcoin/bitcoin/pull/33082)
Remove dead code due to legacy wallet removal.
Leftovers from previous #32481.
💬 achow101 commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#issuecomment-3128506203)
reACK d5104cfbaeb82081e4b00a5084516555e446dcdc
(https://github.com/bitcoin/bitcoin/pull/32279#issuecomment-3128506203)
reACK d5104cfbaeb82081e4b00a5084516555e446dcdc
💬 w0xlt commented on pull request "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver":
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3128518901)
> I don't think it is desirable. OpenSSL is a massive codebase, and we only need a tiny part. We spent a very long time trying to remove OpenSSL as well.
What I meant was reimplementing those OpenSSL functions I mentioned above in our codebase.
> A naive implementation with no proper review could be as bad as using an external library too.
That's my point. Adding cryptographic schemes that aren't directly part of the Bitcoin protocol can be difficult to obtain sufficient review or even
...
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3128518901)
> I don't think it is desirable. OpenSSL is a massive codebase, and we only need a tiny part. We spent a very long time trying to remove OpenSSL as well.
What I meant was reimplementing those OpenSSL functions I mentioned above in our codebase.
> A naive implementation with no proper review could be as bad as using an external library too.
That's my point. Adding cryptographic schemes that aren't directly part of the Bitcoin protocol can be difficult to obtain sufficient review or even
...
🚀 achow101 merged a pull request: "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline"
(https://github.com/bitcoin/bitcoin/pull/32279)
(https://github.com/bitcoin/bitcoin/pull/32279)
📝 pablomartin4btc converted_to_draft a pull request: "wallet, refactor: Remove Legacy check and error"
(https://github.com/bitcoin/bitcoin/pull/33082)
Remove dead code due to legacy wallet removal.
Leftovers from previous #32481.
(https://github.com/bitcoin/bitcoin/pull/33082)
Remove dead code due to legacy wallet removal.
Leftovers from previous #32481.
💬 fanquake commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/33079#issuecomment-3128659016)
Fuzzer is still failing the same as #31367, will drop that out (https://cirrus-ci.com/task/5198999654760448?logs=ci#L5231):
```bash
[13:17:44.505] AddressSanitizer:DEADLYSIGNAL
[13:17:44.505] =================================================================
[13:17:44.505] ==9965==ERROR: AddressSanitizer: stack-overflow on address 0x7ffd88ed3db8 (pc 0x7f91daffff4f bp 0x7ffd88f4fec0 sp 0x7ffd88ed2dc0 T0)
[13:17:44.505] #0 0x7f91daffff4f in std::ostreambuf_iterator<char, std::char_traits<c
...
(https://github.com/bitcoin/bitcoin/pull/33079#issuecomment-3128659016)
Fuzzer is still failing the same as #31367, will drop that out (https://cirrus-ci.com/task/5198999654760448?logs=ci#L5231):
```bash
[13:17:44.505] AddressSanitizer:DEADLYSIGNAL
[13:17:44.505] =================================================================
[13:17:44.505] ==9965==ERROR: AddressSanitizer: stack-overflow on address 0x7ffd88ed3db8 (pc 0x7f91daffff4f bp 0x7ffd88f4fec0 sp 0x7ffd88ed2dc0 T0)
[13:17:44.505] #0 0x7f91daffff4f in std::ostreambuf_iterator<char, std::char_traits<c
...