Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sent_and_received method on Wallet #428

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

reez
Copy link
Collaborator

@reez reez commented Nov 29, 2023

Description

Adds the functionality for sent_and_received

sent_and_received was a new method added in BDK.

Computes total input value going from script pubkeys in the wallet (sent) and the total output value going to script pubkeys in the wallet (received) in the tx passed into the function.

For sent the function needs historical information about the outputs being spent (which are assumed to be indexed), while for received it can rely solely on the tx's outputs without needing prior indexing of spent outputs.

Notes to the reviewers

At the moment I decided to not add a code comment above the sent_and_received function in the wallet.rs file; my thinking was I would like to add a code comment similar to what is in the BDK documentation, but since the BDK documentation is still evolving I wouldn't want to add a code comment in bdk-ffi now but never get back around to updating it as the BDK documentation changes, and thus have a misleading code comment in the future.

Changelog notice

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

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

@reez reez force-pushed the s+r branch 2 times, most recently from e759c9d to 55c36b5 Compare December 1, 2023 17:20
bdk-ffi/src/lib.rs Outdated Show resolved Hide resolved
@reez reez force-pushed the s+r branch 3 times, most recently from 983412e to c97547a Compare December 2, 2023 01:44
@reez reez marked this pull request as ready for review December 4, 2023 16:33
@reez
Copy link
Collaborator Author

reez commented Dec 7, 2023

rebased on master and tests pass

@notmandatory
Copy link
Member

For this function it's safe to go ahead and copy the BDK docs, I don't expect they will change for the 1.0 release.

Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK 46e0324.

I opened a new issue (#431) regarding API docs. I'm ok with not having any in this PR, and a further PR should remove them all so we can do a proper reset.

@thunderbiscuit thunderbiscuit merged commit 46e0324 into bitcoindevkit:master Dec 7, 2023
17 checks passed
@reez reez deleted the s+r branch December 7, 2023 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants