-
-
Notifications
You must be signed in to change notification settings - Fork 295
feat: add balance metadata for snap datasource [DO NOT MERGE] #9564
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ export type AccountBalancesUpdatedEventPayload = { | |
| [assetId: string]: { | ||
| amount: string; | ||
| unit: string; | ||
| metadata?: Record<string, Json>; | ||
| }; | ||
| }; | ||
| }; | ||
|
|
@@ -292,6 +293,7 @@ export class SnapDataSource extends AbstractDataSource< | |
| accountAssets ??= {}; | ||
| accountAssets[assetId as Caip19AssetId] = { | ||
| amount: balance.amount, | ||
| ...(balance.metadata ? { metadata: balance.metadata } : {}), | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when persist balance data via event, we persist metadata together if any type |
||
| }; | ||
| } | ||
| } | ||
|
|
@@ -483,7 +485,7 @@ export class SnapDataSource extends AbstractDataSource< | |
| } | ||
|
|
||
| // Step 2: Get balances for those specific assets | ||
| const balances: Record<CaipAssetType, Balance> = | ||
| const balances: Record<CaipAssetType, Balance & { metadata?: Json }> = | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mock keyring type |
||
| await client.getAccountBalances( | ||
| accountId, | ||
| accountAssets as CaipAssetType[], | ||
|
|
@@ -497,6 +499,7 @@ export class SnapDataSource extends AbstractDataSource< | |
| if (accountBalances) { | ||
| (accountBalances as Record<string, unknown>)[assetId] = { | ||
| amount: balance.amount, | ||
| ...(balance.metadata ? { metadata: balance.metadata } : {}), | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when persist balance data via keyring API get balance, we persist metadata together if any |
||
| }; | ||
| } | ||
| } | ||
|
|
||
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.
add metadata into snap balance update event payload type
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 want type enforced payloads?
E.g. a typed descriminated union?
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.
Or maybe we can enforce a schema for runtime validation & parsing?
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.
i think in future
some snap may still using SNAP path to sync data, those data will not able to predict
adding validation may also adding a complexity on how snap ship data to controller
and it require bump up to controller when the data sharp change..., so i will suggest leave it as this (should align with keyring API)