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: P-1222 added graph getter for OmniAccount in client-sdk #3247

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

higherordertech
Copy link
Contributor

@higherordertech higherordertech commented Feb 2, 2025

Context

As title.

The implementation is replicated from the heima test code:

export const getOmniAccount = async (parachainApi: ApiPromise, identity: CorePrimitivesIdentity): Promise<string> => {
const omniAccount = await parachainApi.rpc.state.call('OmniAccountApi_omni_account', identity.toHex());
return encodeAddress(omniAccount.toHex());
};
export const getOmniAccountNonce = async (

Labels

Please apply following PR-related labels when appropriate:

  • C0-breaking: if your change could break the existing client, e.g. API change, critical logic change
  • C1-noteworthy: if your change is non-breaking, but is still worth noticing for the client, e.g. reference code improvement

How (Optional)

Testing Evidences

Please attach any relevant evidences if applicable

Copy link

linear bot commented Feb 2, 2025

@higherordertech higherordertech requested a review from a team February 2, 2025 23:49

return accountStore.unwrap() as MemberAccount[];
} catch (e) {
// swallow - empty omni graph
Copy link
Member

Choose a reason for hiding this comment

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

Is accountStore.unwrap() the only call that can throw an error ?
If not it might ignore it (for example query error) and return null so user will think that account doesn't exists

* @returns Array of member accounts or null if not found
* @throws Error if API calls fail
*/
export async function getOmniGraph(
Copy link
Contributor

Choose a reason for hiding this comment

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

This getter should probably get the data from the worker (TrustedGetter) as private members are encrypted. But since we are rewriting the id worker in the omni-executor maybe it would be better to wait a bit for this.

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