feat: expose Display trait on Amount and Psbt types by shivv23 · Pull Request #1045 · bitcoindevkit/bdk-ffi

shivv23 · 2026-07-03T08:06:35Z

Description

Add #[uniffi::export(Display)] and implement the Display trait on the Amount and Psbt types, exposing them to the FFI bindings (Kotlin, Swift, Python).

Notes to the reviewers

This follows the same pattern used by other types in the codebase (FeeRate, Script, Address, Transaction, WalletEvent, etc.) that already expose Display to bindings. Part of issue #1020.

Documentation

bitcoin — Amount and Psbt already implement Display upstream.

Changelog

changelog: none

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing
I've added exactly one changelog:* label
I've linked the relevant upstream docs or specs above

New Features:

I've added tests for the new feature
I've added docs for the new feature

j-kon · 2026-07-06T09:42:41Z

Nice small addition, and this seems consistent with the Display exports already used for types like FeeRate, Script, Address, and Transaction.

One small question: should this PR add a focused test for the new binding-facing Display behavior? Since this exposes Display through UniFFI, it would be useful to assert that Amount and Psbt stringify as expected, especially because the PR checklist marks tests as added but the diff only changes bitcoin.rs.

shivv23 · 2026-07-06T17:16:27Z

Hey @j-kon , I should have included tests with the initial commit. I've pushed test_amount_display and test_psbt_display to cover the new Display behavior for both types.

Regarding the changelog label — I have changelog: added in the PR body but it seems the label itself needs to be applied by someone with triage access. Could you add it?

Thanks for the review!

j-kon · 2026-07-07T07:27:16Z

Hey @j-kon , I should have included tests with the initial commit. I've pushed test_amount_display and test_psbt_display to cover the new Display behavior for both types.

Regarding the changelog label — I have changelog: added in the PR body but it seems the label itself needs to be applied by someone with triage access. Could you add it?

Thanks for the review!

Thanks for adding the tests, that addresses my review comment.

I don’t think I have triage access to apply labels on this repo, so I can’t add the changelog: added label myself. Maybe a maintainer can help apply it.

ItoroD

Thanks for adding this. LGTM. Just a few comments commit message for the test should start with

test: add ....

ItoroD · 2026-07-14T13:45:44Z

+#[uniffi::export(Display)]
 pub struct Amount(pub(crate) BdkAmount);

+impl Display for Amount {


Just a nit. Usually we place the display Impl at the bottom of the type implementation. i.e after all implementations of the particular type.

ItoroD · 2026-07-14T13:45:59Z

+#[uniffi::export(Display)]
 pub struct Psbt(pub(crate) Mutex<BdkPsbt>);

+impl Display for Psbt {


shivv23 · 2026-07-14T16:25:56Z

Hi @ItoroD! I have made all changes you mentioned to the PR. Kindly review it again.

ItoroD

On second thought I am thinking we might not need the tests. As in the past we do not typically write tests explicitly for display. This will mean we will have lots of solely display tests for various types which I do not think its necessary. I will just leave this comment here to see what other reviews think.

I am happy to approve one the import change is made.

ItoroD · 2026-07-14T18:23:21Z


+#[test]
+fn test_amount_display() {
+    let amount = crate::bitcoin::Amount::from_sat(100_000_000);


Here, maybe we can use the import at the top of the file like the others. So no need to use the fully qualified import here.

@ItoroD Done!

thunderbiscuit

If you can:

Remove the test
Rebase on master

I think this one is ready to go.

thunderbiscuit · 2026-07-14T19:44:47Z

    );
 }

+#[test]


I agree with @ItoroD; we do not need to add tests for the Display trait.

@thunderbiscuit Agree with you, have made the required changes.

shivv23 · 2026-07-15T07:56:15Z

@ItoroD and @thunderbiscuit Hi! I have made the required changes requested. Thanks!

thunderbiscuit · 2026-07-15T14:22:57Z

I apologize @shivv23 but I didn't notice your commits were not PGP signed. This is a requirement of the BDK Org, so you'll have to sign that commit before I can merge. Otherwise this looks good! (also keep an eye out because if I merge anything in between you'll also have to rebase).

shivv23 · 2026-07-15T20:23:17Z

Hi @thunderbiscuit! All feedback addressed (tests removed, Display placement moved to bottom of type impls, rebased on master). Commit is now GPG-signed and verified. Please take another look.

Signed-off-by: shivv23 <shivamkumar0423@gmail.com>

ItoroD

ACK a62a58b

thunderbiscuit

ACK a62a58b.

github-project-automation Bot added this to BDK-Bindings Jul 3, 2026

shivv23 force-pushed the feat/display-amount-psbt branch from 480f6f1 to 1fd3084 Compare July 3, 2026 08:08

ItoroD added the changelog: none label Jul 8, 2026

ItoroD requested changes Jul 14, 2026

View reviewed changes

shivv23 force-pushed the feat/display-amount-psbt branch 2 times, most recently from c793a9a to 458ca59 Compare July 14, 2026 16:22

ItoroD requested changes Jul 14, 2026

View reviewed changes

thunderbiscuit requested changes Jul 14, 2026

View reviewed changes

shivv23 force-pushed the feat/display-amount-psbt branch from 458ca59 to 342c716 Compare July 15, 2026 07:53

shivv23 force-pushed the feat/display-amount-psbt branch 2 times, most recently from 746203e to ea5036d Compare July 15, 2026 20:22

feat: expose Display trait on Amount and Psbt types

a62a58b

Signed-off-by: shivv23 <shivamkumar0423@gmail.com>

shivv23 force-pushed the feat/display-amount-psbt branch from ea5036d to a62a58b Compare July 15, 2026 20:38

ItoroD approved these changes Jul 16, 2026

View reviewed changes

thunderbiscuit approved these changes Jul 16, 2026

View reviewed changes

thunderbiscuit merged commit a62a58b into bitcoindevkit:master Jul 16, 2026
8 checks passed

github-project-automation Bot moved this to Done in BDK-Bindings Jul 16, 2026

                   );
               }
+              #[test]

Uh oh!

Conversation

shivv23 commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Documentation

Changelog

Checklists

All Submissions:

New Features:

Uh oh!

j-kon commented Jul 6, 2026

Uh oh!

shivv23 commented Jul 6, 2026

Uh oh!

j-kon commented Jul 7, 2026

Uh oh!

ItoroD left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shivv23 commented Jul 14, 2026

Uh oh!

ItoroD left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thunderbiscuit left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shivv23 commented Jul 15, 2026

Uh oh!

thunderbiscuit commented Jul 15, 2026

Uh oh!

shivv23 commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ItoroD left a comment

Choose a reason for hiding this comment

Uh oh!

thunderbiscuit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shivv23 commented Jul 3, 2026 •

edited

Loading

shivv23 commented Jul 15, 2026 •

edited

Loading