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 display of wallet changes to TxSignView #91

Merged
merged 7 commits into from
May 7, 2023

Conversation

danieloravec
Copy link
Contributor

  • Add a TxSignSummary component for displaying
    changes to own wallet. There are two new boxes
    displayed. One green for newly incoming assets and
    one red for assets leaving the wallet.

  • For example, if there are 10 ERG incoming
    to the wallet, but in the same tx, 3 ERG are
    leaving the wallet, then there will be 7 ERG
    displayed in the green box and the red box
    will be empty.

Copy link
Member

@capt-nemo429 capt-nemo429 left a comment

Choose a reason for hiding this comment

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

Some remarks on incoming/leaving assets summary:

  1. Please, move the calculation logic from TxSignSummary.vue to TxInterpreter and expose two properties, namely: totalIncoming and totalLeaving, so that we can only display the summary where it makes sense in the future.
  2. You can make use of utxoSum and utxoSumResultDiff functions from @fleet-sdk/common package to make the code a bit more concise and cleaner.

src/components/TxSignSummary.vue Outdated Show resolved Hide resolved
* Move delta calculation from TxSignSummary
  to txInterpreter.

* Expose new properties totalIncoming and
  totalLeaving in txInterpreter.

* Use utxoSum and utxoSumResultDiff for deltas
  calculation.

* Extract some parts of the calculation to a
  separate utils file under transaction interpreter.

* Some local functions defined in
  _determineChangeBoxes in txInterpreter
  were also needed for _calcIncomingLeavingTotals,
  so there are slight changes to change box
  function as well.
Copy link
Member

@capt-nemo429 capt-nemo429 left a comment

Choose a reason for hiding this comment

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

The implementation looks perfect! Left a small suggestion.

src/components/TxSignSummary.vue Outdated Show resolved Hide resolved
@capt-nemo429 capt-nemo429 merged commit b80b0bf into nautls:add-sign-inputs May 7, 2023
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.

None yet

2 participants