feat: MUSD-248 create money-account-balance-service#8428
Merged
Conversation
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
…mestamp. Vaults aren't guaranteed to have the other properties when there's no activity
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Matt561
commented
Apr 13, 2026
.../money-account-controller/src/money-account-balance-service/money-account-balance-service.ts
Outdated
Show resolved
Hide resolved
…rative fetching of live value (e.g. withdrawal flow)
Matt561
force-pushed
the
feat/musd-248-create-money-account-balance-service
branch
from
dda94fc to
9b51e57
Compare
Contributor
Author
|
@metamaskbot publish-previews |
…account-balance-service
Contributor
Author
|
@metamaskbot publish-previews |
shane-t
previously approved these changes
Apr 14, 2026
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cc71b5c. Configure here.
shane-t
reviewed
Apr 14, 2026
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
mcmire
reviewed
Apr 14, 2026
Contributor
mcmire
left a comment
mcmire
left a comment
There was a problem hiding this comment.
This looks pretty good. Many of the comments below are minor.
|
|
||
| this.#config = { | ||
| ...config, | ||
| vedaApiBaseUrl: config.vedaApiBaseUrl ?? VEDA_PERFORMANCE_API_BASE_URL, |
Contributor
There was a problem hiding this comment.
It looks like this service takes a base URL as an option. Is there a particular reason for this? We don't recommend this because it means that consumers (i.e. some other team, even) may use this class in a way that you didn't intend.
Contributor
Author
There was a problem hiding this comment.
I thought this would be convenient should Veda change their URL (e.g. new version) so we could update via remote flag. I'll remove.
| constructor({ | ||
| messenger, | ||
| policyOptions = {}, | ||
| config, |
Contributor
There was a problem hiding this comment.
Nit: Is there a reason why you are taking a separate set of options here? We recommend that all options for your data service be in the same "options bag". It's true that messenger and policyOptions are passed to BaseDataService, but consumers don't really know that (nor should they). All options should be treated on the same level.
If you want to have a single type for all options, you could create a MoneyAccountBalanceServiceOptions type and add messenger and policyOptions to it.
Contributor
Author
There was a problem hiding this comment.
I thought the separate options bag would be more intuitive but I understand your point. I'll refactor to place all options at the same level.
packages/money-account-balance-service/src/money-account-balance-service.ts
Outdated
Show resolved
Hide resolved
|
|
||
| const rawResponse = await response.json(); | ||
|
|
||
| // Validate raw response inside queryFn to avoid poisoned cache. |
Contributor
There was a problem hiding this comment.
Hmm, this is a point we didn't consider when creating the data service examples. I guess that does make sense.
| * const { balance } = await service.getMusdBalance('0xYourMoneyAccount...'); | ||
| * ``` | ||
| */ | ||
| export class MoneyAccountBalanceService extends BaseDataService< |
Contributor
There was a problem hiding this comment.
Since this talks to the Veda API would it make more sense to be literal and call this VedaApiService? Or do we expect to wrap this API in the future?
Contributor
Author
There was a problem hiding this comment.
Good question. I ultimately picked this name to better convey intent/context. If it's okay, I'd prefer to leave as is for now.
| */ | ||
| export function normalizeVaultApyResponse( | ||
| rawResponse: Infer<typeof VaultApyResponseStruct>, | ||
| ): VaultApyResponse { |
Contributor
There was a problem hiding this comment.
Nit: It might be confusing that VaultApyResponse and Infer<typeof VaultApyResponseStruct> are not the same thing.
Contributor
Author
There was a problem hiding this comment.
I've renamed VaultApyResponseStruct to VaultApyRawResponseStruct and VaultApyResponse to NormalizedVaultApyResponse. I'd like to differentiate between the raw response structure we get from Veda's API and our internal usage.
| @@ -0,0 +1,22 @@ | |||
| export { | |||
| MoneyAccountBalanceService, | |||
| serviceName as moneyAccountBalanceServiceName, | |||
Contributor
There was a problem hiding this comment.
The name of the service is not conventionally exported, can you remove this?
Suggested change
| serviceName as moneyAccountBalanceServiceName, |
Contributor
Author
There was a problem hiding this comment.
This is something I copied from the SocialService here
I will remove though if that's preferred.
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
mcmire
approved these changes
Apr 14, 2026
Contributor
mcmire
left a comment
mcmire
left a comment
There was a problem hiding this comment.
Two more comments, but they are non-blocking. LGTM!
Comment on lines
+269
to
+272
| afterEach(() => { | ||
| jest.resetAllMocks(); | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
Nit: This happens in our test setup already, so this should not be necessary:
Suggested change
| afterEach(() => { | |
| jest.resetAllMocks(); | |
| }); |
| describe('MoneyAccountBalanceService', () => { | ||
| beforeEach(() => { | ||
| MockWeb3Provider.mockImplementation(() => ({}) as unknown as Web3Provider); | ||
| nockCleanAll(); |
Contributor
There was a problem hiding this comment.
Nit: This happens in our test setup already, so this should not be necessary:
Suggested change
| nockCleanAll(); |
Contributor
Author
|
Triple-checking that functionality works in Mobile before merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Adds
MoneyAccountBalanceServiceto@metamask/money-account-controllerto support the MetaMask Money feature (mUSD stablecoin and musdSHFvd Veda vault shares). The service exposes five messenger actions: fetching mUSD and musdSHFvd ERC-20 balances via RPC, reading the Veda Accountant contract exchange rate, computing mUSD-equivalent vault share value, and fetching vault APY from Veda's Seven Seas REST API. All on-chain reads use@ethersproject/contractswith aWeb3Providerresolved throughNetworkControllermessenger actions.Key design decisions
getMusdEquivalentValuedelegates to the two underlying cached queries and performs BigInt arithmetic over cached results rather than issuing its ownfetchQuery, avoiding cache coordination issues.VedaResponseValidationErrorexcluded from retry policy: Malformed API responses are a deterministic failure — retrying them wastes quota. A custom error class signals the service policy to skip retry for validation failures.type()rather thanobject()so unknown fields don't cause validation failures, guarding against future API additions without breaking the service.@metamask/utilspromoted to production dependency: Previously a dev dependency, it is now required at runtime for hex/BigInt utilities used in balance and exchange rate computation.References
Checklist
Note
Medium Risk
Adds a new cached data service that performs on-chain RPC reads and external REST fetches; correctness and error-handling/caching behavior could impact displayed balances/APY, but changes are largely additive and isolated to a new package.
Overview
Introduces a new package,
@metamask/money-account-balance-service, implementingMoneyAccountBalanceService(aBaseDataService) that exposes messenger actions to fetch mUSD and vault-share ERC-20 balances, read the Veda Accountant exchange rate, compute mUSD-equivalent vault-share value, and fetch/normalize vault APY from Veda’s Seven Seas REST API (with Superstruct response validation and a non-retryableVedaResponseValidationError).Wires the new package into repo metadata (adds to
READMEpackage list/dependency graph,CODEOWNERS,teams.json, andyarn.lock) and includes full package scaffolding (build/test/typedoc configs, changelog, and extensive Jest coverage including caching and error-path behavior).Reviewed by Cursor Bugbot for commit 2deb59b. Bugbot is set up for automated code reviews on this repo. Configure here.