-
Notifications
You must be signed in to change notification settings - Fork 120
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
Improve clarity on asset mint and universe stats RPC responses #553
Conversation
Thanks so much for these updates! I'll test it out locally, but at first glance, this is looking great 👍 |
I ran the same script from #522 using this branch. The assets stats output looks great now. 👌 One minor quirk I noticed is that the output of View CLI commands & output
I also wrote another script to mint multiple tranches of normal assets in order to see how the stats output is reflected in this case. The only problem I've found here is in the View CLI commands & output
Since the asset stats are now grouped by |
78f0a4e
to
ac16ee0
Compare
@jamaljsr thanks again for the detailed tests and info, really helpful!
That's fixed in the latest version.
I added the number of tranches as well now. This should currently be the same number as the number of proofs, as we only store issuance proofs. But when we start storing transfer proofs those numbers will start to diverge, so I think it's good to have that separate number.
I'm not sure the universe stats call is the right one for that? Since this is supposed to be somewhat condensed. You could still look up all assets within a group, since you have the group key in the result. |
ac16ee0
to
92623d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really good!
IIUC NumTranches
is now the count of all issuance events, including ungrouped assets?
This PR now depends on #556, as it made more sense to group all bug fixes into a single PR. |
8825f57
to
e2637b8
Compare
e2637b8
to
9229fe9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 💯
Will def need to have these fields well-documented somewhere.
9229fe9
to
cc203b1
Compare
What does a tranche represent? I thought it meant that if I mint an asset, then wait sometime and mint more of that same asset, then wait some time and mint more, this would be considered 3 tranches. In my testing, the Mint 3 assets with the same group_key over time
Mint 5 distinct assets on the same node as above
|
Yeah that's correct, we're planning on renaming that to just |
We want to be able to match the group key with universe roots, which use the x-only (32-byte) part of the group key. To make the matching easier, we add a x_only_group_key version of the tweaked group key to the key_group_info_view.
cc203b1
to
975a969
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🥐
Fixes #522.
Depends on #556.Addresses the following items:
TapCommitment
for grouped assets which we fix)QueryAssetStats
by putting them into different fields (see below)Example output of
tapcli universe stats
:Example output of
tapcli universe stats assets --sort_by asset_name
:cc @jamaljsr