cashubtc/cdk
1
0
Fork 0
Code Issues 25 Pull requests 23 Projects Releases 43 Packages Wiki Activity Actions

feat: bip321 request parsing and creation #1716

Open
thesimplekid wants to merge 1 commit from thesimplekid/bip_321 into main
pull from: thesimplekid/bip_321
merge into: cashubtc:main
Conversation 4 Commits 1 Files changed 23 +1724 -550
thesimplekid commented 2026-03-07 12:56:16 +00:00 (Migrated from github.com)
Copy link

Description

Notes to the reviewers

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

### Description <!-- Describe the purpose of this PR, what's being adding and/or fixed --> ----- ### Notes to the reviewers <!-- In this section you can include notes directed to the reviewers, like explaining why some parts of the PR were done in a specific way --> ----- ### Suggested [CHANGELOG](https://github.com/cashubtc/cdk/blob/main/CHANGELOG.md) Updates <!-- Please do not edit the actual changelog but note what you changed here. --> #### CHANGED #### ADDED #### REMOVED #### FIXED ---- ### Checklist * [ ] I followed the [code style guidelines](https://github.com/cashubtc/cdk/blob/main/CODE_STYLE.md) * [ ] I ran `just final-check` before committing
cdk-bot commented 2026-03-09 22:54:54 +00:00 (Migrated from github.com)
Copy link

@codex review

@codex review
chatgpt-codex-connector[bot] (Migrated from github.com) reviewed 2026-03-09 22:59:57 +00:00
chatgpt-codex-connector[bot] (Migrated from github.com) left a comment
Copy link

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d379a268eb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `d379a268eb` <details> <summary>ℹ️ About Codex in GitHub</summary> <br/> [Your team has set up Codex to review pull requests in this repo](http://chatgpt.com/codex/settings/general). Reviews are triggered when you - Open a pull request for review - Mark a draft as ready - Comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". </details>
crates/cdk/src/wallet/bip321.rs
@ -0,0 +1,571 @@
//! BIP 321 Payment Instruction Helpers
chatgpt-codex-connector[bot] (Migrated from github.com) commented 2026-03-09 22:59:57 +00:00
Copy link

P1 Badge Use parsed instruction network instead of hardcoding mainnet

The parser always calls PaymentInstructions::parse with bitcoin::Network::Bitcoin, so valid testnet/regtest payment instructions (e.g., tb1... addresses or non-mainnet Lightning invoices) will be rejected even though this API is exposed as a general wallet helper. This makes parse_payment_instruction unusable in non-mainnet environments and can break integration/regtest workflows; the network should be inferred from input or passed in by the caller rather than fixed to mainnet.

Useful? React with 👍 / 👎.

**<sub><sub>![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat)</sub></sub> Use parsed instruction network instead of hardcoding mainnet** The parser always calls `PaymentInstructions::parse` with `bitcoin::Network::Bitcoin`, so valid testnet/regtest payment instructions (e.g., `tb1...` addresses or non-mainnet Lightning invoices) will be rejected even though this API is exposed as a general wallet helper. This makes `parse_payment_instruction` unusable in non-mainnet environments and can break integration/regtest workflows; the network should be inferred from input or passed in by the caller rather than fixed to mainnet. Useful? React with 👍 / 👎.
thesimplekid commented 2026-03-17 21:46:01 +00:00 (Migrated from github.com)
Copy link

@codex review

@codex review
chatgpt-codex-connector[bot] (Migrated from github.com) reviewed 2026-03-17 21:54:25 +00:00
chatgpt-codex-connector[bot] (Migrated from github.com) left a comment
Copy link

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a748bcb37b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `a748bcb37b` <details> <summary>ℹ️ About Codex in GitHub</summary> <br/> [Your team has set up Codex to review pull requests in this repo](http://chatgpt.com/codex/settings/general). Reviews are triggered when you - Open a pull request for review - Mark a draft as ready - Comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". </details>
crates/cdk-cli/src/sub_commands/melt.rs
chatgpt-codex-connector[bot] (Migrated from github.com) commented 2026-03-17 21:54:25 +00:00
Copy link

P2 Badge Avoid hardcoding mainnet for BIP353 quote parsing

The CLI now always passes bitcoin::Network::Bitcoin for BIP353 melt quotes, so parsing will reject valid non-mainnet on-chain methods embedded in resolved BIP321 instructions (e.g., tb1/bcrt1 fallbacks). Since cdk-cli has no network override here, users operating against testnet/regtest-style deployments can’t request BIP353 quotes when records include those addresses.

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Avoid hardcoding mainnet for BIP353 quote parsing** The CLI now always passes `bitcoin::Network::Bitcoin` for BIP353 melt quotes, so parsing will reject valid non-mainnet on-chain methods embedded in resolved BIP321 instructions (e.g., `tb1`/`bcrt1` fallbacks). Since `cdk-cli` has no network override here, users operating against testnet/regtest-style deployments can’t request BIP353 quotes when records include those addresses. Useful? React with 👍 / 👎.
crates/cdk/src/wallet/bip321.rs
@ -0,0 +1,571 @@
//! BIP 321 Payment Instruction Helpers
chatgpt-codex-connector[bot] (Migrated from github.com) commented 2026-03-17 21:54:25 +00:00
Copy link

P1 Badge Preserve post-resolution parse errors instead of resolve errors

melt_human_readable_quote only falls back to Lightning Address when it sees Error::Bip353Resolve, but this mapping currently converts any parse failure of an already-resolved bitcoin: URI into Bip353Resolve. In practice, a BIP353 DNS record that resolves successfully but contains an invalid/unusable BOLT12 payload will be treated as a resolution failure and silently trigger LNURL fallback, which contradicts the no-fallback-on-unusable-BOLT12 behavior and can change payment routing for addresses that publish both mechanisms.

Useful? React with 👍 / 👎.

**<sub><sub>![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat)</sub></sub> Preserve post-resolution parse errors instead of resolve errors** `melt_human_readable_quote` only falls back to Lightning Address when it sees `Error::Bip353Resolve`, but this mapping currently converts any parse failure of an already-resolved `bitcoin:` URI into `Bip353Resolve`. In practice, a BIP353 DNS record that resolves successfully but contains an invalid/unusable BOLT12 payload will be treated as a resolution failure and silently trigger LNURL fallback, which contradicts the no-fallback-on-unusable-BOLT12 behavior and can change payment routing for addresses that publish both mechanisms. Useful? React with 👍 / 👎.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin thesimplekid/bip_321:thesimplekid/bip_321
git switch thesimplekid/bip_321

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff thesimplekid/bip_321
git switch thesimplekid/bip_321
git rebase main
git switch main
git merge --ff-only thesimplekid/bip_321
git switch thesimplekid/bip_321
git rebase main
git switch main
git merge --no-ff thesimplekid/bip_321
git switch main
git merge --squash thesimplekid/bip_321
git switch main
git merge --ff-only thesimplekid/bip_321
git switch main
git merge thesimplekid/bip_321
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
DB & Storage

   
Deployment

   
Error Handling & Logging

   
Maintenance

   
Payment Backend

   
backport

   
backport v0.13.x

   
backport v0.14.x

   
backport v0.15.x

   
bindings

   
blocked

   
bug

Something isn't working

  
cdk-sql

   
ci

   
cli

   
deps

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
keep-open

   
ldk-node-ui

   
migrations

   
mint

   
mutation-testing

   
needs rebase

   
needs review

   
new nut

This PR is held pending the merge of a new or updated NUT

  
nut change

   
question

Further information is requested

  
ready

   
rust-version

   
rustfmt

   
stacked hold

PR is based on top of another PR, so should be merged in sequence. See comments for details.

  
stale

   
testing

   
wallet

   
weekly-report

   
wontfix

This will not be worked on

No labels
DB & Storage
Deployment
Error Handling & Logging
Maintenance
Payment Backend
backport
backport v0.13.x
backport v0.14.x
backport v0.15.x
bindings
blocked
bug
cdk-sql
ci
cli
deps
documentation
duplicate
enhancement
good first issue
help wanted
invalid
keep-open
ldk-node-ui
migrations
mint
mutation-testing
needs rebase
needs review
new nut
nut change
question
ready
rust-version
rustfmt
stacked hold
stale
testing
wallet
weekly-report
wontfix
Milestone
Clear milestone
No items
No milestone
0.16.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cashubtc/cdk!1716
No description provided.