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: make account parser implementation extendable #1530

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

0xpatrickdev
Copy link

@0xpatrickdev 0xpatrickdev commented Dec 22, 2023

  • adds AccountParserManager class that enables registering AccountParsers
  • adds createAccountParserRegistry, a map of well-known typeUrls and AccountParser's
  • preserves existing accountFromAny functionality

- adds AccountParserManager class where consumers can register different accountParsers
- adds createDefaultAccountParser() which instantiates AccountParserManager with common cosmos-sdk account types
- preserves existing accountFromAny interface for backwards compatability
- refactor createDefaultAccountParser (instance) to createAccountParserRegistry (map of accountParsers) for easier testing
Comment on lines +134 to +136
* const myAccountParserManager = new AccountParserManager(createAccountParserRegistry());
* myAccountParserManager.register('/custom.type.v1.CustomAccount', customAccountParser);
* const account = customParserManager.parseAccount(someInput);
Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
* const myAccountParserManager = new AccountParserManager(createAccountParserRegistry());
* myAccountParserManager.register('/custom.type.v1.CustomAccount', customAccountParser);
* const account = customParserManager.parseAccount(someInput);
* const { register, parseAccount } = new AccountParserManager(createAccountParserRegistry());
* register('/custom.type.v1.CustomAccount', customAccountParser);
* const options: StargateClientOptions = { accountParser: parseAccount };

This probably aligns closer to expected usage

@@ -40,49 +40,103 @@ function accountFromBaseAccount(input: BaseAccount): Account {
*/
export type AccountParser = (any: Any) => Account;

export type AccountParserRegistry = Map<Any["typeUrl"], AccountParser>;
Copy link
Author

@0xpatrickdev 0xpatrickdev Dec 23, 2023

Choose a reason for hiding this comment

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

Realizing we may also want to export accountFromBaseAccount (L26-L35), as this will likely be needed by those supplying a custom account parser. Alternatively, this could be baked into every parseAccount call as every AccountParser function likely needs this as the last step.

Will wait for feedback on this and the overall design before making any updates.

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.

1 participant