-
Notifications
You must be signed in to change notification settings - Fork 229
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 ChainAddress type #9662
Conversation
Deploying agoric-sdk with
|
Latest commit: |
33f92b5
|
Status: | ✅ Deploy successful! |
Preview URL: | https://1930c541.agoric-sdk.pages.dev |
Branch Preview URL: | https://9162-chainaddress.agoric-sdk.pages.dev |
156ce76
to
d79552f
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. The CI failure is surprising as it seems unrelated
@@ -22,7 +22,7 @@ import type { AmountArg, ChainAddress, DenomAmount } from './types.js'; | |||
|
|||
/** An address for a validator on some blockchain, e.g., cosmos, eth, etc. */ | |||
export type CosmosValidatorAddress = ChainAddress & { | |||
// TODO document why this is the format | |||
// infix for Validator Operator https://docs.cosmos.network/main/learn/beginner/accounts#addresses |
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.
We might consider 1) noting this is valoper
conventionally, 2) moving this value to ChainInfo
keyed as bech32PrefixValPub
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. I also like the change to encoding
rather than addressEncoding
- fixes changed parameter from #9662 - improves test to print delegate offer result error on failure
- fixes changed parameter from #9662 - improves test to print delegate offer result error on failure
- fixes changed parameter from #9662 - improves test to print delegate offer result error on failure
- fixes changed parameter from #9662 - improves test to print delegate offer result error on failure
closes: #9162
Description
#9162 is already mostly done. This revisits the property names with some improvements. Also cleans up nearby types and docs.
#9162 (comment) has some good ideas for future directions but this PR wraps up the requirements for release.
Security Considerations
Scaling Considerations
Documentation Considerations
Testing Considerations
Upgrade Considerations
unreleased