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

feat: Add the parse function to extract common elements of the tx #213

Merged
merged 26 commits into from
Jul 11, 2022

Conversation

taro04
Copy link
Contributor

@taro04 taro04 commented Jun 11, 2022

@YasunoriMATSUOKA

Please review.
I extracted the information from tx.
7/15 types of tx were confirmed. I haven't reached any other types of tx.
I think it's enough to check with some types of tx.

But if you need to check for other types of tx, please tell me how to access that tx or the block height that contains the tx.
(Nothing appeared in the [gov] pull-down of explorer / transaction page.)

image

@kimurayu45z
Copy link
Contributor

I think that...
using the table with same table heads, is unreasonable.
If there is some transaction type which is difficult to adapt this table heads (and I think there are many such transaction types), this UI will lead to be confusing or unreasonable UI.

So, the logic for parsing tx types is good. The remaining big problem is UI, I think.

@taro04
Copy link
Contributor Author

taro04 commented Jun 11, 2022

It is not possible to use a table with the same table head.

I think so. now, I still feel a lot of difficulty. (Especially MsgWithdrawDelegatorReward )

Then, should I change the table head to an item common to Tx?
For example, BlockHeight, (Hash), MessageType, Time.

@YasunoriMATSUOKA YasunoriMATSUOKA changed the title fea: Add the parse function to extract common elements of the tx feat: Add the parse function to extract common elements of the tx Jun 13, 2022
@taro04
Copy link
Contributor Author

taro04 commented Jun 13, 2022

@YasunoriMATSUOKA, @kimurayu45z

I would like to change the table header like this.

  • tx in block page
    image
  • tx in account page
    image
    (Do you think need ”Height” here?)

Please let us know if you have any other ideas.

@taro04 taro04 self-assigned this Jun 13, 2022
@taro04
Copy link
Contributor Author

taro04 commented Jun 14, 2022

@YasunoriMATSUOKA, cc @kimurayu45z

Please, review. 🙏
After that, I will implement parsing of the tx detail page.

  • tx in address page
    image

  • tx in block page
    image

@taro04
Copy link
Contributor Author

taro04 commented Jun 21, 2022

WIP

  • distribution
    20220621_distribution

  • gov
    20220621_gov

  • staking & send
    20220621_staking send

Todo:
parse proposal.content
Some Tx do not match their type (MsgVoteWeighted/MsgCreateVestingAccount/MsgSetWithdrawAddress)

@taro04
Copy link
Contributor Author

taro04 commented Jun 28, 2022

@YasunoriMATSUOKA
I checked all type of tx in detail page.
Please review. 🙏

debug on chainID = 'ununifi-alpha-test-v2'

debug on chainID = 'create-validator-debug';

However, I think this PR will be merged after conmos-client v0.46.0 is completely merged.

@YasunoriMATSUOKA
Copy link
Contributor

@taro04
Sorry, can you fix marketplace CI errors? (Unnecessary to debug marketplace (and shared) features. Only necessary to fix CI errors(=build errors).)

@taro04
Copy link
Contributor Author

taro04 commented Jun 29, 2022

Sorry, unintentionally modifying the marketplace was also the cause of the CI error.

The build error is gone, but I'm updating Angular.json in this PR, so I think this PR should be merged after #236 merges.

@YasunoriMATSUOKA YasunoriMATSUOKA changed the title feat: Add the parse function to extract common elements of the tx [WIP] feat: Add the parse function to extract common elements of the tx Jul 5, 2022
@YasunoriMATSUOKA YasunoriMATSUOKA changed the title [WIP] feat: Add the parse function to extract common elements of the tx feat: Add the parse function to extract common elements of the tx Jul 11, 2022
Copy link
Contributor

@YasunoriMATSUOKA YasunoriMATSUOKA left a comment

Choose a reason for hiding this comment

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

LGTM

@YasunoriMATSUOKA YasunoriMATSUOKA merged commit b24f392 into develop Jul 11, 2022
@YasunoriMATSUOKA YasunoriMATSUOKA deleted the feature/parse-tx-info branch July 11, 2022 03:44
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