Skip to content

feat: expose Display trait on Amount and Psbt types#1045

Open
shivv23 wants to merge 2 commits into
bitcoindevkit:masterfrom
shivv23:feat/display-amount-psbt
Open

feat: expose Display trait on Amount and Psbt types#1045
shivv23 wants to merge 2 commits into
bitcoindevkit:masterfrom
shivv23:feat/display-amount-psbt

Conversation

@shivv23

@shivv23 shivv23 commented Jul 3, 2026

Copy link
Copy Markdown

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

Changelog

changelog: added

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

Signed-off-by: shivv23 <shivamkumar0423@gmail.com>
@shivv23 shivv23 force-pushed the feat/display-amount-psbt branch from 480f6f1 to 1fd3084 Compare July 3, 2026 08:08
@j-kon

j-kon commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

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

shivv23 commented Jul 6, 2026

Copy link
Copy Markdown
Author

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!

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants