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 transaction summary to wallet:post #4910

Merged
merged 3 commits into from
Apr 20, 2024
Merged

Add transaction summary to wallet:post #4910

merged 3 commits into from
Apr 20, 2024

Conversation

dguenther
Copy link
Member

@dguenther dguenther commented Apr 18, 2024

Summary

There isn't an easy way to view the contents of a rawTransaction now except by starting up the repl and deserializing it. I think it's pretty expected that the wallet:post command should show a transaction summary, since our other commands already show something like that before the transaction is finalized.

Examples

Sending

===================
Transaction Summary
===================

From            e00eea42e3dcc19fa7ef00814c1f8a6688e641a297aff2448fbe5b4b3780d15c
Fee             $IRON 0.00000002
Expiration      475838

==================
Spends (2)
==================

Asset           $IRON
Note Hash       1346b34ac395d16422e728520f4491467ad0acc2257759a1e50fb0d47d0eb729

------------------

Asset           47e8540a5e2b72ecd1cd09d6c13c4af5ec8d7d02a8da5787f8153f2dd2f0387d
Note Hash       f65c7e60e945876e9a4741da1d7d7c6eeebc08661b603cdcdfc4e70c93d8ac30


==================
Notes (1) (Additional notes will be added to return unspent assets to the sender)
==================

Amount         50
Asset          47e8540a5e2b72ecd1cd09d6c13c4af5ec8d7d02a8da5787f8153f2dd2f0387d
Memo           test memo
Recipient      d3920422e4e4833324a5b3e980f92ebb7d2d1672a78f78dcd9386ba40da20af1
Sender         e00eea42e3dcc19fa7ef00814c1f8a6688e641a297aff2448fbe5b4b3780d15c


Do you want to post this (Y/N)?:

Minting

===================
Transaction Summary
===================

From            e00eea42e3dcc19fa7ef00814c1f8a6688e641a297aff2448fbe5b4b3780d15c
Fee             $IRON 0.00000001
Expiration      475838

==================
Mints (1)
==================

Amount         50
Asset ID       403e8affdb858e56fb874aca2da3004952c1cafd87f19d47922bed1ab0d8377e
Name           testasset
Metadata       no


==================
Spends (1)
==================

Asset           $IRON
Note Hash       1346b34ac395d16422e728520f4491467ad0acc2257759a1e50fb0d47d0eb729


Do you want to post this (Y/N)?:

Testing Plan

  • Ran wallet:post with output of wallet:mint --rawTransaction and wallet:send --rawTransaction

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.

[ ] Yes

Breaking Change

Is this a breaking change? If yes, add notes below on why this is breaking and label it with breaking-change-rpc or breaking-change-sdk.

[ ] Yes

@dguenther dguenther requested a review from a team as a code owner April 18, 2024 19:01
@patnir
Copy link
Contributor

patnir commented Apr 18, 2024

when i try to send 2 iron from an account i get the following output:

image

maybe it would be clearer if the spends also included to/ from information so that i know that the funds are coming back to me in this case?

@patnir
Copy link
Contributor

patnir commented Apr 18, 2024

Another thing that would be helpful is to see how much IRON is being used for gas

@dguenther
Copy link
Member Author

maybe it would be clearer if the spends also included to/ from information so that i know that the funds are coming back to me in this case?

To/From on spends isn't really relevant to using them. I'll change spends to just show the note hash and asset for now, since it's confusing

Another thing that would be helpful is to see how much IRON is being used for gas

This should be in there, it's the fee in the transaction summary.

@dguenther
Copy link
Member Author

I changed around the formatting a bit and updated the description.

@patnir
Copy link
Contributor

patnir commented Apr 19, 2024

Why do spends not have a value here?

@patnir
Copy link
Contributor

patnir commented Apr 19, 2024

Why not show a number for spends? Example:

===================
Transaction Summary
===================

From            505a7c3e0b3c62444aa7070a96f556d517a26e9dc12487563030de614c877849
Fee             $IRON 0.00000005
Expiration      2625

==================
Spends (10)
==================

Asset:          $IRON
Note Hash:      00086b93e3521a8a673baec4748170cde8a4c89cfa3168707a4b5ccee15c2244

------------------

Asset:          $IRON
Note Hash:      0040b49bb17f3c3405a376d6e34788571eddfccf32115ce37259c8c1a95b9915

------------------

Asset:          $IRON
Note Hash:      00597701aa2b740a7e22719654407de4d1f9d2760ad144143f078338ea8f6302

------------------

Asset:          $IRON
Note Hash:      0060afd39f9c5b3c25d651d01ad6219d4b51ac4f9771333172a6b2424fcdaf3f

------------------

Asset:          $IRON
Note Hash:      007c7d0ab0817c3905cf8d703a4672c2536b8691aea6430b2bb52c31a8f64608

------------------

Asset:          $IRON
Note Hash:      0085a9349b5c06194ce443a71a03946cc78258a9f2289ae869f9b02181563b22

------------------

Asset:          $IRON
Note Hash:      00b7d6a359148d96e1e4e11731dadb6839938d59df01115a17adb6c350ec6e41

------------------

Asset:          $IRON
Note Hash:      00c03fd84ae032774ec7bca290440e5ea8d5073221649295a6aaec9adbab364b

------------------

Asset:          $IRON
Note Hash:      010198abf31e55ee6c1ca29ae574ef65910b100b55c465e11e1c13588db9ec0c

------------------

Asset:          $IRON
Note Hash:      01265b8b6e420fd5b41b86a47bc0e3ad75b3c866f5c2f973751f8dd7798c4310


==================
Notes (1) (Additional notes will be added to return unspent assets to the sender)
==================

Amount:        $IRON 199.99999995
Memo:          asdf
Recipient:     505a7c3e0b3c62444aa7070a96f556d517a26e9dc12487563030de614c877849
Sender:        505a7c3e0b3c62444aa7070a96f556d517a26e9dc12487563030de614c877849


Do you want to post this (Y/N)?: 

@patnir patnir force-pushed the add-post-summary branch from e06fe13 to a92dd78 Compare April 19, 2024 15:23
@dguenther
Copy link
Member Author

Can you clarify what information you'd like to have added/removed that users might want to see in this summary? I'm primarily trying to address the issue that the CLI doesn't provide a way to view the contents of a raw transaction (other than writing a repl command). I know viewing raw notes/spends can have UX issues, we've run into those with the node app/mobile wallet, but I'd rather default to showing as much of the underlying RawTransaction data as possible without being too confusing

@dguenther dguenther enabled auto-merge (squash) April 20, 2024 17:01
@dguenther dguenther merged commit 036bdaa into staging Apr 20, 2024
6 checks passed
@dguenther dguenther deleted the add-post-summary branch April 20, 2024 17:12
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.

2 participants