-
Notifications
You must be signed in to change notification settings - Fork 19
feat: format known addresses #616
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
base: master
Are you sure you want to change the base?
feat: format known addresses #616
Conversation
✅ Deploy Preview for oasisprotocol-cli canceled.
|
0068b35 to
95ace6e
Compare
|
This would be nice addition. @uniyalabhishek can you rebase? @matevz can you review please? |
@ptrus sorry, this PR is not fully ready for review now (also oasisprotocol/oasis-sdk#2335). there have been higher-priority tasks and missed completing this. will prioritise it this week. |
95ace6e to
ee0dbb3
Compare
|
FYI I'll be refactoring |
c451ac0 to
a2b7ccc
Compare
matevz
left a comment
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.
First pass. IMO we should simplify more.
cmd/common/helpers.go
Outdated
| func GenAddressFormatContextForNetwork(net *configSdk.Network) AddressFormatContext { | ||
| return AddressFormatContext{ | ||
| Names: GenAccountNamesForNetwork(net), | ||
| Eth: GenAccountEthMapForNetwork(net), |
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.
Do we need to separate between the eth and non-eth accounts?
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.
no, the ETH map is just optional metadata keyed by native address. If an ETH hex is known for that address we prefer it, otherwise we fall back to native bech32.
fix(account show): propagate derived eth address
a2b7ccc to
f0e7240
Compare
This makes “known address” rendering consistent across CLI text output, and prefers Ethereum hex in parentheses when available (while still showing both addresses where required).
oasis wallet showandoasis account showdisplay both Ethereum + native addresses, elsewhere prefer Ethereum in parentheses when available.Dependency: This PR depends on oasis-sdk PR oasisprotocol/oasis-sdk#2335 being merged and a new
github.com/oasisprotocol/oasis-sdk/client-sdk/gorelease being published, after that we’ll bump the SDK version.