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.

Sign up for GitHub

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

Jump to bottom

feat(ironfish): add spends to wallet/getTransactions #3834

Merged
merged 2 commits into from
Apr 25, 2023
Merged

feat(ironfish): add spends to wallet/getTransactions #3834

merged 2 commits into from
Apr 25, 2023

Conversation

holahula
Copy link
Contributor

@holahula holahula commented Apr 24, 2023 •
edited
Loading

Summary

Adds spends to wallet/getTransactions

Closes IFL-736

Testing Plan

Updated existing unit tests

Documentation

Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference)? If yes, link a
related documentation pull request for the website.

[ X] Yes

iron-fish/website#400

Breaking Change

Is this a breaking change? If yes, add notes below on why this is breaking and
what additional work is required, if any.

[ ] Yes

@holahula holahula mentioned this pull request Apr 24, 2023
Merged
@holahula holahula changed the title feat(ironfish): add spends to wallet/getTransactions feat(ironfish): add spends to wallet/getTransactions Apr 24, 2023
@holahula holahula marked this pull request as ready for review April 24, 2023 21:17
@holahula holahula requested a review from a team as a code owner April 24, 2023 21:17
@holahula holahula force-pushed the holahula/feat/wallet-account-txn-vin branch from f67d656 to 326735f Compare April 24, 2023 22:00
@holahula holahula mentioned this pull request Apr 24, 2023
Merged
ygao76
ygao76 reviewed Apr 24, 2023
View reviewed changes
Copy link
Contributor

@ygao76 ygao76 left a comment

Choose a reason for hiding this comment

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

looks good, can you add the response spends check in other existing unit tests in getTransaciont.test.ts

eg.

 expect(transactions[0].spends).toEqual(block.transactions[0].getSpend(0).map{...})

ygao76
ygao76 approved these changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@ygao76 ygao76 left a comment

Choose a reason for hiding this comment

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

Approve with the condition the comment is addressed

@holahula holahula force-pushed the holahula/feat/wallet-account-txn-vin branch from 326735f to b3d99af Compare April 25, 2023 15:12
@holahula holahula merged commit c34de43 into staging Apr 25, 2023
@holahula holahula deleted the holahula/feat/wallet-account-txn-vin branch April 25, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NullSoldier NullSoldier NullSoldier left review comments

@ygao76 ygao76 ygao76 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@holahula @NullSoldier @ygao76