wip(payjoin-send): failing to sign the original PSBT - #191

Closed
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send
Closed

wip(payjoin-send): failing to sign the original PSBT#191
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send

Conversation

@mehmetefeumit

Copy link
Copy Markdown
Contributor

Description

This PR implements Payjoin sending functionality to the BDK CLI.

Draft Explanation

At the time of writing, this is a draft PR for implementing SendPayjoin to the BDK CLI. It still needs session persistence to be implemented, but I've been focused on the Payjoin receiver and sender commands so that I will work on later.

I also have a ReceivePayjoin branch on my local, but I am currently stuck at signing for both Sender and Receiver, so I am using this draft PR to get help regarding signing.

Regarding the current version of the PR, I need help with signing the original PSBT.

Notes to the reviewers

A couple of things I'd need more recommendation on:

  1. Is the current way of handling the errors (through mapping them to a generic BDKCLI error) the best way to handle it? It makes the code quite verbose, but I was not able to find a generic Payjoin error type to add to the BDKCLIError enum so that I can just ? throughout the code.
  2. I cannot get the signing for the original PSBT right. I am currently testing through a regtest on my local, and using the payjoin-cli for the receiver end. When I generate the URI with payjoin-cli receive and use that in the sending logic I've implemented here, the receiver returns the following. This does not have any witness data at all, and the finalized does return false. I currently cannot understand why it cannot sign the UTXOs owned by this wallet though...:
02000000014df3998e42e1fc60baa75a3bd16458d15d8842f7409c42c2d7498d1b8e945f590000000000fdffffff0210270000000000001600144fe208a8c68474e8d147804bface77057fc5ecdf63d10295000000001600149ef197c3deb0a655c92df6964306aeb96a1c991ef9010000
Error: Replied with error: Can't broadcast. PSBT rejected by mempool.

Changelog notice

I'm leaving the checklist to later since this is a draft PR for the purpose of asking for assistance.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@DanGould

Copy link
Copy Markdown

I love seeing this advance!

Error Handling

Much like how rust-bitcoin does not have a single generic error type because there are multiple errors to handle differently, rust-bitcoin does not have a generic error type. There are many errors that may need different handling. Some are recoverable and won't need to be printed. Some might be terminal. Our next update will make this easier, but for now, the payjoin-cli reference implementation is even incomplete with regard to error handling. If you want to create an single enum with variants for each of the payjoin error types, that could work. I'm not sure what BDK-CLI's error handling strategy is.

I'd want to tap another BDK-CLI contributor to figure out what the preference for error handling for this project is.

Receiver Signing

In order to sign, I think you'll need to re-introduce the UTXO data to the PSBT so BDK can figure out what keys go to each input since that data was stripped by the receiver. You can see that I faced a similar issue, and how I resolved it using BDK 1.0 alpha in Mutiny way back. I did the same with BitMask. This is a limitation of BDK.

https://github.com/MutinyWallet/mutiny-node/pull/647/files#diff-c9658b4aca938566e0b3f07bb42c10b33aa59c479c6e174d5cf840f6432bcd1aR489-R496

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15279732847

Details

  • 0 of 91(0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 2.461%

Changes Missing CoverageCovered LinesChanged/Added Lines%
src/commands.rs020.0%
src/utils.rs070.0%
src/handlers.rs0820.0%
Files with Coverage ReductionNew Missed Lines%
src/handlers.rs13.34%
TotalsCoverage Status
Change from base Build 15127343813:-0.2%
Covered Lines:25
Relevant Lines:1016

💛 - Coveralls

@notmandatorynotmandatory added the enhancement New feature or request label May 28, 2025
@notmandatorynotmandatory mentioned this pull request May 28, 2025
@DanGouldDanGould moved this from Backlog to In Progress in Payjoin Roadmap 📝May 28, 2025
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in Payjoin Roadmap 📝Jun 3, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in BDK-CLIJun 3, 2025
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit

Copy link
Copy Markdown
ContributorAuthor

Accidentally closed. Please see the finalized PR: #200

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@mehmetefeumit@DanGould@coveralls@notmandatory
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

wip(payjoin-send): failing to sign the original PSBT - #191

Closed
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send
Closed

wip(payjoin-send): failing to sign the original PSBT#191
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send

Conversation

@mehmetefeumit

Copy link
Copy Markdown
Contributor

Description

This PR implements Payjoin sending functionality to the BDK CLI.

Draft Explanation

At the time of writing, this is a draft PR for implementing SendPayjoin to the BDK CLI. It still needs session persistence to be implemented, but I've been focused on the Payjoin receiver and sender commands so that I will work on later.

I also have a ReceivePayjoin branch on my local, but I am currently stuck at signing for both Sender and Receiver, so I am using this draft PR to get help regarding signing.

Regarding the current version of the PR, I need help with signing the original PSBT.

Notes to the reviewers

A couple of things I'd need more recommendation on:

  1. Is the current way of handling the errors (through mapping them to a generic BDKCLI error) the best way to handle it? It makes the code quite verbose, but I was not able to find a generic Payjoin error type to add to the BDKCLIError enum so that I can just ? throughout the code.
  2. I cannot get the signing for the original PSBT right. I am currently testing through a regtest on my local, and using the payjoin-cli for the receiver end. When I generate the URI with payjoin-cli receive and use that in the sending logic I've implemented here, the receiver returns the following. This does not have any witness data at all, and the finalized does return false. I currently cannot understand why it cannot sign the UTXOs owned by this wallet though...:
02000000014df3998e42e1fc60baa75a3bd16458d15d8842f7409c42c2d7498d1b8e945f590000000000fdffffff0210270000000000001600144fe208a8c68474e8d147804bface77057fc5ecdf63d10295000000001600149ef197c3deb0a655c92df6964306aeb96a1c991ef9010000
Error: Replied with error: Can't broadcast. PSBT rejected by mempool.

Changelog notice

I'm leaving the checklist to later since this is a draft PR for the purpose of asking for assistance.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@DanGould

Copy link
Copy Markdown

I love seeing this advance!

Error Handling

Much like how rust-bitcoin does not have a single generic error type because there are multiple errors to handle differently, rust-bitcoin does not have a generic error type. There are many errors that may need different handling. Some are recoverable and won't need to be printed. Some might be terminal. Our next update will make this easier, but for now, the payjoin-cli reference implementation is even incomplete with regard to error handling. If you want to create an single enum with variants for each of the payjoin error types, that could work. I'm not sure what BDK-CLI's error handling strategy is.

I'd want to tap another BDK-CLI contributor to figure out what the preference for error handling for this project is.

Receiver Signing

In order to sign, I think you'll need to re-introduce the UTXO data to the PSBT so BDK can figure out what keys go to each input since that data was stripped by the receiver. You can see that I faced a similar issue, and how I resolved it using BDK 1.0 alpha in Mutiny way back. I did the same with BitMask. This is a limitation of BDK.

https://github.com/MutinyWallet/mutiny-node/pull/647/files#diff-c9658b4aca938566e0b3f07bb42c10b33aa59c479c6e174d5cf840f6432bcd1aR489-R496

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15279732847

Details

  • 0 of 91(0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 2.461%

Changes Missing CoverageCovered LinesChanged/Added Lines%
src/commands.rs020.0%
src/utils.rs070.0%
src/handlers.rs0820.0%
Files with Coverage ReductionNew Missed Lines%
src/handlers.rs13.34%
TotalsCoverage Status
Change from base Build 15127343813:-0.2%
Covered Lines:25
Relevant Lines:1016

💛 - Coveralls

@notmandatorynotmandatory added the enhancement New feature or request label May 28, 2025
@notmandatorynotmandatory mentioned this pull request May 28, 2025
@DanGouldDanGould moved this from Backlog to In Progress in Payjoin Roadmap 📝May 28, 2025
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in Payjoin Roadmap 📝Jun 3, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in BDK-CLIJun 3, 2025
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit

Copy link
Copy Markdown
ContributorAuthor

Accidentally closed. Please see the finalized PR: #200

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@mehmetefeumit@DanGould@coveralls@notmandatory
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

wip(payjoin-send): failing to sign the original PSBT - #191

Closed
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send
Closed

wip(payjoin-send): failing to sign the original PSBT#191
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send

Conversation

@mehmetefeumit

Copy link
Copy Markdown
Contributor

Description

This PR implements Payjoin sending functionality to the BDK CLI.

Draft Explanation

At the time of writing, this is a draft PR for implementing SendPayjoin to the BDK CLI. It still needs session persistence to be implemented, but I've been focused on the Payjoin receiver and sender commands so that I will work on later.

I also have a ReceivePayjoin branch on my local, but I am currently stuck at signing for both Sender and Receiver, so I am using this draft PR to get help regarding signing.

Regarding the current version of the PR, I need help with signing the original PSBT.

Notes to the reviewers

A couple of things I'd need more recommendation on:

  1. Is the current way of handling the errors (through mapping them to a generic BDKCLI error) the best way to handle it? It makes the code quite verbose, but I was not able to find a generic Payjoin error type to add to the BDKCLIError enum so that I can just ? throughout the code.
  2. I cannot get the signing for the original PSBT right. I am currently testing through a regtest on my local, and using the payjoin-cli for the receiver end. When I generate the URI with payjoin-cli receive and use that in the sending logic I've implemented here, the receiver returns the following. This does not have any witness data at all, and the finalized does return false. I currently cannot understand why it cannot sign the UTXOs owned by this wallet though...:
02000000014df3998e42e1fc60baa75a3bd16458d15d8842f7409c42c2d7498d1b8e945f590000000000fdffffff0210270000000000001600144fe208a8c68474e8d147804bface77057fc5ecdf63d10295000000001600149ef197c3deb0a655c92df6964306aeb96a1c991ef9010000
Error: Replied with error: Can't broadcast. PSBT rejected by mempool.

Changelog notice

I'm leaving the checklist to later since this is a draft PR for the purpose of asking for assistance.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@DanGould

Copy link
Copy Markdown

I love seeing this advance!

Error Handling

Much like how rust-bitcoin does not have a single generic error type because there are multiple errors to handle differently, rust-bitcoin does not have a generic error type. There are many errors that may need different handling. Some are recoverable and won't need to be printed. Some might be terminal. Our next update will make this easier, but for now, the payjoin-cli reference implementation is even incomplete with regard to error handling. If you want to create an single enum with variants for each of the payjoin error types, that could work. I'm not sure what BDK-CLI's error handling strategy is.

I'd want to tap another BDK-CLI contributor to figure out what the preference for error handling for this project is.

Receiver Signing

In order to sign, I think you'll need to re-introduce the UTXO data to the PSBT so BDK can figure out what keys go to each input since that data was stripped by the receiver. You can see that I faced a similar issue, and how I resolved it using BDK 1.0 alpha in Mutiny way back. I did the same with BitMask. This is a limitation of BDK.

https://github.com/MutinyWallet/mutiny-node/pull/647/files#diff-c9658b4aca938566e0b3f07bb42c10b33aa59c479c6e174d5cf840f6432bcd1aR489-R496

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15279732847

Details

  • 0 of 91(0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 2.461%

Changes Missing CoverageCovered LinesChanged/Added Lines%
src/commands.rs020.0%
src/utils.rs070.0%
src/handlers.rs0820.0%
Files with Coverage ReductionNew Missed Lines%
src/handlers.rs13.34%
TotalsCoverage Status
Change from base Build 15127343813:-0.2%
Covered Lines:25
Relevant Lines:1016

💛 - Coveralls

@notmandatorynotmandatory added the enhancement New feature or request label May 28, 2025
@notmandatorynotmandatory mentioned this pull request May 28, 2025
@DanGouldDanGould moved this from Backlog to In Progress in Payjoin Roadmap 📝May 28, 2025
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in Payjoin Roadmap 📝Jun 3, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in BDK-CLIJun 3, 2025
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit

Copy link
Copy Markdown
ContributorAuthor

Accidentally closed. Please see the finalized PR: #200

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@mehmetefeumit@DanGould@coveralls@notmandatory
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

wip(payjoin-send): failing to sign the original PSBT - #191

Closed
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send
Closed

wip(payjoin-send): failing to sign the original PSBT#191
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send

Conversation

@mehmetefeumit

Copy link
Copy Markdown
Contributor

Description

This PR implements Payjoin sending functionality to the BDK CLI.

Draft Explanation

At the time of writing, this is a draft PR for implementing SendPayjoin to the BDK CLI. It still needs session persistence to be implemented, but I've been focused on the Payjoin receiver and sender commands so that I will work on later.

I also have a ReceivePayjoin branch on my local, but I am currently stuck at signing for both Sender and Receiver, so I am using this draft PR to get help regarding signing.

Regarding the current version of the PR, I need help with signing the original PSBT.

Notes to the reviewers

A couple of things I'd need more recommendation on:

  1. Is the current way of handling the errors (through mapping them to a generic BDKCLI error) the best way to handle it? It makes the code quite verbose, but I was not able to find a generic Payjoin error type to add to the BDKCLIError enum so that I can just ? throughout the code.
  2. I cannot get the signing for the original PSBT right. I am currently testing through a regtest on my local, and using the payjoin-cli for the receiver end. When I generate the URI with payjoin-cli receive and use that in the sending logic I've implemented here, the receiver returns the following. This does not have any witness data at all, and the finalized does return false. I currently cannot understand why it cannot sign the UTXOs owned by this wallet though...:
02000000014df3998e42e1fc60baa75a3bd16458d15d8842f7409c42c2d7498d1b8e945f590000000000fdffffff0210270000000000001600144fe208a8c68474e8d147804bface77057fc5ecdf63d10295000000001600149ef197c3deb0a655c92df6964306aeb96a1c991ef9010000
Error: Replied with error: Can't broadcast. PSBT rejected by mempool.

Changelog notice

I'm leaving the checklist to later since this is a draft PR for the purpose of asking for assistance.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@DanGould

Copy link
Copy Markdown

I love seeing this advance!

Error Handling

Much like how rust-bitcoin does not have a single generic error type because there are multiple errors to handle differently, rust-bitcoin does not have a generic error type. There are many errors that may need different handling. Some are recoverable and won't need to be printed. Some might be terminal. Our next update will make this easier, but for now, the payjoin-cli reference implementation is even incomplete with regard to error handling. If you want to create an single enum with variants for each of the payjoin error types, that could work. I'm not sure what BDK-CLI's error handling strategy is.

I'd want to tap another BDK-CLI contributor to figure out what the preference for error handling for this project is.

Receiver Signing

In order to sign, I think you'll need to re-introduce the UTXO data to the PSBT so BDK can figure out what keys go to each input since that data was stripped by the receiver. You can see that I faced a similar issue, and how I resolved it using BDK 1.0 alpha in Mutiny way back. I did the same with BitMask. This is a limitation of BDK.

https://github.com/MutinyWallet/mutiny-node/pull/647/files#diff-c9658b4aca938566e0b3f07bb42c10b33aa59c479c6e174d5cf840f6432bcd1aR489-R496

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15279732847

Details

  • 0 of 91(0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 2.461%

Changes Missing CoverageCovered LinesChanged/Added Lines%
src/commands.rs020.0%
src/utils.rs070.0%
src/handlers.rs0820.0%
Files with Coverage ReductionNew Missed Lines%
src/handlers.rs13.34%
TotalsCoverage Status
Change from base Build 15127343813:-0.2%
Covered Lines:25
Relevant Lines:1016

💛 - Coveralls

@notmandatorynotmandatory added the enhancement New feature or request label May 28, 2025
@notmandatorynotmandatory mentioned this pull request May 28, 2025
@DanGouldDanGould moved this from Backlog to In Progress in Payjoin Roadmap 📝May 28, 2025
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in Payjoin Roadmap 📝Jun 3, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in BDK-CLIJun 3, 2025
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit

Copy link
Copy Markdown
ContributorAuthor

Accidentally closed. Please see the finalized PR: #200

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@mehmetefeumit@DanGould@coveralls@notmandatory
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

wip(payjoin-send): failing to sign the original PSBT - #191

Closed
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send
Closed

wip(payjoin-send): failing to sign the original PSBT#191
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send

Conversation

@mehmetefeumit

Copy link
Copy Markdown
Contributor

Description

This PR implements Payjoin sending functionality to the BDK CLI.

Draft Explanation

At the time of writing, this is a draft PR for implementing SendPayjoin to the BDK CLI. It still needs session persistence to be implemented, but I've been focused on the Payjoin receiver and sender commands so that I will work on later.

I also have a ReceivePayjoin branch on my local, but I am currently stuck at signing for both Sender and Receiver, so I am using this draft PR to get help regarding signing.

Regarding the current version of the PR, I need help with signing the original PSBT.

Notes to the reviewers

A couple of things I'd need more recommendation on:

  1. Is the current way of handling the errors (through mapping them to a generic BDKCLI error) the best way to handle it? It makes the code quite verbose, but I was not able to find a generic Payjoin error type to add to the BDKCLIError enum so that I can just ? throughout the code.
  2. I cannot get the signing for the original PSBT right. I am currently testing through a regtest on my local, and using the payjoin-cli for the receiver end. When I generate the URI with payjoin-cli receive and use that in the sending logic I've implemented here, the receiver returns the following. This does not have any witness data at all, and the finalized does return false. I currently cannot understand why it cannot sign the UTXOs owned by this wallet though...:
02000000014df3998e42e1fc60baa75a3bd16458d15d8842f7409c42c2d7498d1b8e945f590000000000fdffffff0210270000000000001600144fe208a8c68474e8d147804bface77057fc5ecdf63d10295000000001600149ef197c3deb0a655c92df6964306aeb96a1c991ef9010000
Error: Replied with error: Can't broadcast. PSBT rejected by mempool.

Changelog notice

I'm leaving the checklist to later since this is a draft PR for the purpose of asking for assistance.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@DanGould

Copy link
Copy Markdown

I love seeing this advance!

Error Handling

Much like how rust-bitcoin does not have a single generic error type because there are multiple errors to handle differently, rust-bitcoin does not have a generic error type. There are many errors that may need different handling. Some are recoverable and won't need to be printed. Some might be terminal. Our next update will make this easier, but for now, the payjoin-cli reference implementation is even incomplete with regard to error handling. If you want to create an single enum with variants for each of the payjoin error types, that could work. I'm not sure what BDK-CLI's error handling strategy is.

I'd want to tap another BDK-CLI contributor to figure out what the preference for error handling for this project is.

Receiver Signing

In order to sign, I think you'll need to re-introduce the UTXO data to the PSBT so BDK can figure out what keys go to each input since that data was stripped by the receiver. You can see that I faced a similar issue, and how I resolved it using BDK 1.0 alpha in Mutiny way back. I did the same with BitMask. This is a limitation of BDK.

https://github.com/MutinyWallet/mutiny-node/pull/647/files#diff-c9658b4aca938566e0b3f07bb42c10b33aa59c479c6e174d5cf840f6432bcd1aR489-R496

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15279732847

Details

  • 0 of 91(0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 2.461%

Changes Missing CoverageCovered LinesChanged/Added Lines%
src/commands.rs020.0%
src/utils.rs070.0%
src/handlers.rs0820.0%
Files with Coverage ReductionNew Missed Lines%
src/handlers.rs13.34%
TotalsCoverage Status
Change from base Build 15127343813:-0.2%
Covered Lines:25
Relevant Lines:1016

💛 - Coveralls

@notmandatorynotmandatory added the enhancement New feature or request label May 28, 2025
@notmandatorynotmandatory mentioned this pull request May 28, 2025
@DanGouldDanGould moved this from Backlog to In Progress in Payjoin Roadmap 📝May 28, 2025
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in Payjoin Roadmap 📝Jun 3, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in BDK-CLIJun 3, 2025
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit

Copy link
Copy Markdown
ContributorAuthor

Accidentally closed. Please see the finalized PR: #200

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@mehmetefeumit@DanGould@coveralls@notmandatory
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

wip(payjoin-send): failing to sign the original PSBT - #191

Closed
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send
Closed

wip(payjoin-send): failing to sign the original PSBT#191
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send

Conversation

@mehmetefeumit

Copy link
Copy Markdown
Contributor

Description

This PR implements Payjoin sending functionality to the BDK CLI.

Draft Explanation

At the time of writing, this is a draft PR for implementing SendPayjoin to the BDK CLI. It still needs session persistence to be implemented, but I've been focused on the Payjoin receiver and sender commands so that I will work on later.

I also have a ReceivePayjoin branch on my local, but I am currently stuck at signing for both Sender and Receiver, so I am using this draft PR to get help regarding signing.

Regarding the current version of the PR, I need help with signing the original PSBT.

Notes to the reviewers

A couple of things I'd need more recommendation on:

  1. Is the current way of handling the errors (through mapping them to a generic BDKCLI error) the best way to handle it? It makes the code quite verbose, but I was not able to find a generic Payjoin error type to add to the BDKCLIError enum so that I can just ? throughout the code.
  2. I cannot get the signing for the original PSBT right. I am currently testing through a regtest on my local, and using the payjoin-cli for the receiver end. When I generate the URI with payjoin-cli receive and use that in the sending logic I've implemented here, the receiver returns the following. This does not have any witness data at all, and the finalized does return false. I currently cannot understand why it cannot sign the UTXOs owned by this wallet though...:
02000000014df3998e42e1fc60baa75a3bd16458d15d8842f7409c42c2d7498d1b8e945f590000000000fdffffff0210270000000000001600144fe208a8c68474e8d147804bface77057fc5ecdf63d10295000000001600149ef197c3deb0a655c92df6964306aeb96a1c991ef9010000
Error: Replied with error: Can't broadcast. PSBT rejected by mempool.

Changelog notice

I'm leaving the checklist to later since this is a draft PR for the purpose of asking for assistance.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@DanGould

Copy link
Copy Markdown

I love seeing this advance!

Error Handling

Much like how rust-bitcoin does not have a single generic error type because there are multiple errors to handle differently, rust-bitcoin does not have a generic error type. There are many errors that may need different handling. Some are recoverable and won't need to be printed. Some might be terminal. Our next update will make this easier, but for now, the payjoin-cli reference implementation is even incomplete with regard to error handling. If you want to create an single enum with variants for each of the payjoin error types, that could work. I'm not sure what BDK-CLI's error handling strategy is.

I'd want to tap another BDK-CLI contributor to figure out what the preference for error handling for this project is.

Receiver Signing

In order to sign, I think you'll need to re-introduce the UTXO data to the PSBT so BDK can figure out what keys go to each input since that data was stripped by the receiver. You can see that I faced a similar issue, and how I resolved it using BDK 1.0 alpha in Mutiny way back. I did the same with BitMask. This is a limitation of BDK.

https://github.com/MutinyWallet/mutiny-node/pull/647/files#diff-c9658b4aca938566e0b3f07bb42c10b33aa59c479c6e174d5cf840f6432bcd1aR489-R496

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15279732847

Details

  • 0 of 91(0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 2.461%

Changes Missing CoverageCovered LinesChanged/Added Lines%
src/commands.rs020.0%
src/utils.rs070.0%
src/handlers.rs0820.0%
Files with Coverage ReductionNew Missed Lines%
src/handlers.rs13.34%
TotalsCoverage Status
Change from base Build 15127343813:-0.2%
Covered Lines:25
Relevant Lines:1016

💛 - Coveralls

@notmandatorynotmandatory added the enhancement New feature or request label May 28, 2025
@notmandatorynotmandatory mentioned this pull request May 28, 2025
@DanGouldDanGould moved this from Backlog to In Progress in Payjoin Roadmap 📝May 28, 2025
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in Payjoin Roadmap 📝Jun 3, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in BDK-CLIJun 3, 2025
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit

Copy link
Copy Markdown
ContributorAuthor

Accidentally closed. Please see the finalized PR: #200

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@mehmetefeumit@DanGould@coveralls@notmandatory
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

wip(payjoin-send): failing to sign the original PSBT - #191

Closed
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send
Closed

wip(payjoin-send): failing to sign the original PSBT#191
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send

Conversation

@mehmetefeumit

Copy link
Copy Markdown
Contributor

Description

This PR implements Payjoin sending functionality to the BDK CLI.

Draft Explanation

At the time of writing, this is a draft PR for implementing SendPayjoin to the BDK CLI. It still needs session persistence to be implemented, but I've been focused on the Payjoin receiver and sender commands so that I will work on later.

I also have a ReceivePayjoin branch on my local, but I am currently stuck at signing for both Sender and Receiver, so I am using this draft PR to get help regarding signing.

Regarding the current version of the PR, I need help with signing the original PSBT.

Notes to the reviewers

A couple of things I'd need more recommendation on:

  1. Is the current way of handling the errors (through mapping them to a generic BDKCLI error) the best way to handle it? It makes the code quite verbose, but I was not able to find a generic Payjoin error type to add to the BDKCLIError enum so that I can just ? throughout the code.
  2. I cannot get the signing for the original PSBT right. I am currently testing through a regtest on my local, and using the payjoin-cli for the receiver end. When I generate the URI with payjoin-cli receive and use that in the sending logic I've implemented here, the receiver returns the following. This does not have any witness data at all, and the finalized does return false. I currently cannot understand why it cannot sign the UTXOs owned by this wallet though...:
02000000014df3998e42e1fc60baa75a3bd16458d15d8842f7409c42c2d7498d1b8e945f590000000000fdffffff0210270000000000001600144fe208a8c68474e8d147804bface77057fc5ecdf63d10295000000001600149ef197c3deb0a655c92df6964306aeb96a1c991ef9010000
Error: Replied with error: Can't broadcast. PSBT rejected by mempool.

Changelog notice

I'm leaving the checklist to later since this is a draft PR for the purpose of asking for assistance.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@DanGould

Copy link
Copy Markdown

I love seeing this advance!

Error Handling

Much like how rust-bitcoin does not have a single generic error type because there are multiple errors to handle differently, rust-bitcoin does not have a generic error type. There are many errors that may need different handling. Some are recoverable and won't need to be printed. Some might be terminal. Our next update will make this easier, but for now, the payjoin-cli reference implementation is even incomplete with regard to error handling. If you want to create an single enum with variants for each of the payjoin error types, that could work. I'm not sure what BDK-CLI's error handling strategy is.

I'd want to tap another BDK-CLI contributor to figure out what the preference for error handling for this project is.

Receiver Signing

In order to sign, I think you'll need to re-introduce the UTXO data to the PSBT so BDK can figure out what keys go to each input since that data was stripped by the receiver. You can see that I faced a similar issue, and how I resolved it using BDK 1.0 alpha in Mutiny way back. I did the same with BitMask. This is a limitation of BDK.

https://github.com/MutinyWallet/mutiny-node/pull/647/files#diff-c9658b4aca938566e0b3f07bb42c10b33aa59c479c6e174d5cf840f6432bcd1aR489-R496

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15279732847

Details

  • 0 of 91(0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 2.461%

Changes Missing CoverageCovered LinesChanged/Added Lines%
src/commands.rs020.0%
src/utils.rs070.0%
src/handlers.rs0820.0%
Files with Coverage ReductionNew Missed Lines%
src/handlers.rs13.34%
TotalsCoverage Status
Change from base Build 15127343813:-0.2%
Covered Lines:25
Relevant Lines:1016

💛 - Coveralls

@notmandatorynotmandatory added the enhancement New feature or request label May 28, 2025
@notmandatorynotmandatory mentioned this pull request May 28, 2025
@DanGouldDanGould moved this from Backlog to In Progress in Payjoin Roadmap 📝May 28, 2025
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in Payjoin Roadmap 📝Jun 3, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in BDK-CLIJun 3, 2025
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit

Copy link
Copy Markdown
ContributorAuthor

Accidentally closed. Please see the finalized PR: #200

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@mehmetefeumit@DanGould@coveralls@notmandatory
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

wip(payjoin-send): failing to sign the original PSBT - #191

Closed
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send
Closed

wip(payjoin-send): failing to sign the original PSBT#191
mehmetefeumit wants to merge 1 commit into
bitcoindevkit:masterfrom
mehmetefeumit:payjoin-send

Conversation

@mehmetefeumit

Copy link
Copy Markdown
Contributor

Description

This PR implements Payjoin sending functionality to the BDK CLI.

Draft Explanation

At the time of writing, this is a draft PR for implementing SendPayjoin to the BDK CLI. It still needs session persistence to be implemented, but I've been focused on the Payjoin receiver and sender commands so that I will work on later.

I also have a ReceivePayjoin branch on my local, but I am currently stuck at signing for both Sender and Receiver, so I am using this draft PR to get help regarding signing.

Regarding the current version of the PR, I need help with signing the original PSBT.

Notes to the reviewers

A couple of things I'd need more recommendation on:

  1. Is the current way of handling the errors (through mapping them to a generic BDKCLI error) the best way to handle it? It makes the code quite verbose, but I was not able to find a generic Payjoin error type to add to the BDKCLIError enum so that I can just ? throughout the code.
  2. I cannot get the signing for the original PSBT right. I am currently testing through a regtest on my local, and using the payjoin-cli for the receiver end. When I generate the URI with payjoin-cli receive and use that in the sending logic I've implemented here, the receiver returns the following. This does not have any witness data at all, and the finalized does return false. I currently cannot understand why it cannot sign the UTXOs owned by this wallet though...:
02000000014df3998e42e1fc60baa75a3bd16458d15d8842f7409c42c2d7498d1b8e945f590000000000fdffffff0210270000000000001600144fe208a8c68474e8d147804bface77057fc5ecdf63d10295000000001600149ef197c3deb0a655c92df6964306aeb96a1c991ef9010000
Error: Replied with error: Can't broadcast. PSBT rejected by mempool.

Changelog notice

I'm leaving the checklist to later since this is a draft PR for the purpose of asking for assistance.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@DanGould

Copy link
Copy Markdown

I love seeing this advance!

Error Handling

Much like how rust-bitcoin does not have a single generic error type because there are multiple errors to handle differently, rust-bitcoin does not have a generic error type. There are many errors that may need different handling. Some are recoverable and won't need to be printed. Some might be terminal. Our next update will make this easier, but for now, the payjoin-cli reference implementation is even incomplete with regard to error handling. If you want to create an single enum with variants for each of the payjoin error types, that could work. I'm not sure what BDK-CLI's error handling strategy is.

I'd want to tap another BDK-CLI contributor to figure out what the preference for error handling for this project is.

Receiver Signing

In order to sign, I think you'll need to re-introduce the UTXO data to the PSBT so BDK can figure out what keys go to each input since that data was stripped by the receiver. You can see that I faced a similar issue, and how I resolved it using BDK 1.0 alpha in Mutiny way back. I did the same with BitMask. This is a limitation of BDK.

https://github.com/MutinyWallet/mutiny-node/pull/647/files#diff-c9658b4aca938566e0b3f07bb42c10b33aa59c479c6e174d5cf840f6432bcd1aR489-R496

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15279732847

Details

  • 0 of 91(0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 2.461%

Changes Missing CoverageCovered LinesChanged/Added Lines%
src/commands.rs020.0%
src/utils.rs070.0%
src/handlers.rs0820.0%
Files with Coverage ReductionNew Missed Lines%
src/handlers.rs13.34%
TotalsCoverage Status
Change from base Build 15127343813:-0.2%
Covered Lines:25
Relevant Lines:1016

💛 - Coveralls

@notmandatorynotmandatory added the enhancement New feature or request label May 28, 2025
@notmandatorynotmandatory mentioned this pull request May 28, 2025
@DanGouldDanGould moved this from Backlog to In Progress in Payjoin Roadmap 📝May 28, 2025
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in Payjoin Roadmap 📝Jun 3, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in BDK-CLIJun 3, 2025
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit restored the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit
mehmetefeumit deleted the payjoin-send branch June 3, 2025 16:04
@mehmetefeumit

Copy link
Copy Markdown
ContributorAuthor

Accidentally closed. Please see the finalized PR: #200

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@mehmetefeumit@DanGould@coveralls@notmandatory