Ramps Controller Foundation #7091

pkowalski · 2025-11-07T16:12:29Z

Explanation

Current State:

Ramps currently depends on an sdk to handle all connections to the RAMPS api. We now have 2 different offerings which each use their own SDK. All of this routing logic is done on the mobile app. If this logic were to be extended to mobile or extension it would require rework on those codebases.

Why we want to change things:

Many other teams already use this controller design pattern. By moving towards this we can handle all business logic in 1 controller. It lets us expose Ramps data and apis to other experiences without those experiences requiring any knowledge of business logic. Lastly it would speed up any Ramps api integrations on extension and mobile in the future.

Changes introduced:

This is the foundation for the Ramps Controller. It contains 1 function which updates its internal state which reflects eligibility for a ramps feature on a certain region.

References

Related to TRAM-2783

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

Note

Introduce @metamask/ramps-controller with a getCountries action that resolves API base by environment, fetches geolocation and region eligibility, and updates controller state.

New Package: packages/ramps-controller
- Controller: RampsController with state (metamaskEnvironment, context, region) and metadata exposure.
- API Logic: getCountries fetches geolocation then regions/countries/<code> using env-specific base URL; validates response and updates state.region; registers messenger action RampsController:getCountries.
- Types/Exports: Public types and enums (SdkEnvironment, Context) via src/index.ts.
- Tests: Comprehensive unit tests for env URL selection, success path, validation, error handling, state updates, and messenger action registration.
- Build/Docs: Jest config, TypeScript configs, Typedoc config, package metadata, README, LICENSE, CHANGELOG.
Repo Integration:
- Add @metamask/ramps-controller to README.md package list and dependency graph.
- Update tsconfig.json and tsconfig.build.json references.
- Update .github/CODEOWNERS to add Ramp team ownership and release file ownership entries.
- Add workspace entry in yarn.lock.

^{Written by Cursor Bugbot for commit 9efa238. This will update automatically on new commits. Configure here.}

tsconfig.build.json

cursor · 2025-11-07T16:18:59Z

packages/ramps-controller/src/RampsController.ts

+      console.error('Error fetching geolocation:', error);
+      throw error;
+    }
+  }


Bug: Unsafe geolocation: non-string API data breaks URLs

The #getGeolocation method returns the API response data without validating it's actually a string. If the API returns a non-string value (like an object or number), it will be used directly in URL construction and stored in state.region.id, causing malformed URLs and incorrect state. Runtime validation should confirm the response is a string before returning it.

socket-security · 2025-11-07T16:24:05Z

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

mcmire

Hello, I took at this controller and had a few notes. Let me know what you think.

In particular, I don't know if you have seen this, but we have a sample-controllers repo that serves as our source of truth for how controllers ought to look (for example, here is an example controller and here are its tests). You might go through this and make sure you're doing things in a similar way.

mcmire · 2025-11-07T18:30:06Z

packages/ramps-controller/src/RampsController.ts

+      messenger,
+      name: controllerName,
+      metadata: rampsControllerMetadata,
+      state: { ...defaultState, ...state } as RampsControllerState,


Is there a reason why you are using a type assertion here? There should be no need for this, you should have a complete state object here.

mcmire · 2025-11-07T18:30:47Z

packages/ramps-controller/src/RampsController.ts

+ */
+export type RampsControllerState = {
+  // Environment tells us which API urls to us
+  metamaskEnvironment: SdkEnvironment;


Why is the metamaskEnvironment and context in state? Do we expect to be able to change these at runtime?

context also doesn't seem to be getting used anywhere.

Good point. Will move context and environment out of state.

Context will be used for future calls. I will add it to the axios object as a default param.

mcmire · 2025-11-07T19:25:51Z

packages/ramps-controller/src/RampsController.ts

+      }
+
+      this.update((state) => {
+        state.region = {


Can you explain how you intend to use this controller? Do you expect to call getCountries() somewhere and listen for changes to region somewhere else? Why not call it immediately and get the results?

True, we can do both. Return it but update the state. We will want to access it later without calling the function so keeping it in state was intentional.

I added a getter method which will init the state if not initialized. Let me know if this makes sense or if the patter is to access the state in the controller directly instead.

Hmm, I don't see this, is it possible you forgot to push those changes?

mcmire · 2025-11-07T19:29:51Z

packages/ramps-controller/src/RampsController.ts

+  async #getGeolocation(): Promise<string> {
+    try {
+      const url = this.#getApiUrl();
+      const response = await fetch(new URL('geolocation', url).toString());


Instead of making HTTP requests in your controller, we recommend extracting an API client from your controller that represents the "on ramp" API. It could serve as a direct layer on top of the layer and could be used independent of the controller anywhere in the stack.

We have a pattern called "data services" that would be useful here. In this case perhaps you want to:

Create a separate file, perhaps src/OnRampService.ts.

Define an OnRampService class in it. The constructor should take two arguments: messenger and policyOptions. It might also be best to take environment and/or context options based on what you're already doing in the controller (the environment can be used to construct the base URL).

Add getGeolocation and getCountries methods to the class, similar to how you have here. Each method should make a fetch and that fetch should be rapped in a policy.execute call.

Add a messenger option to the constructor. Create a OnRampServiceMessenger type and expose those methods using the messenger (see other comment on how to do this in a more automatic way)

Add the service messenger actions to the RampsController's messenger, and then call the service methods through the messenger (e.g. await messenger.call('OnRampService:getGeolocation', ...).

We have an example data service here: https://github.com/MetaMask/core/blob/e144cc58d7b82c0d75c685de85adc3de510a1b3e/packages/sample-controllers/src/sample-gas-prices-service/sample-gas-prices-service.ts. And you can see how it's used in the controller here: https://github.com/MetaMask/core/blob/e144cc58d7b82c0d75c685de85adc3de510a1b3e/packages/sample-controllers/src/sample-gas-prices-service/sample-gas-prices-service.ts

Let me know your thoughts on this!

mcmire · 2025-11-07T19:32:02Z

packages/ramps-controller/src/RampsController.ts

+  },
+};
+
+const defaultState: RampsControllerState = {


Have you read our controller guidelines? This states that the default state should be wrapped in a function: https://github.com/MetaMask/core/blob/main/docs/controller-guidelines.md#provide-a-default-representation-of-state

mcmire · 2025-11-07T19:33:06Z

packages/ramps-controller/src/RampsController.ts

+        state.region = null;
+      });
+      // Or rethrow if you want the caller to handle it:
+      // throw error;


Generally in controllers and services we don't recommend swallowing errors like this unless there is a good reason to do so. But it depends on the use case — can you provide an example of how this would be used in the client?

mcmire · 2025-11-07T19:35:48Z

packages/ramps-controller/src/RampsController.ts

+      state: { ...defaultState, ...state } as RampsControllerState,
+    });
+
+    this.#registerMessageHandlers();


A little while ago we added a way to automatically expose methods as actions. This way you don't need to think about and maintain the types so much whenever you add new methods in the future. Here are some instructions:

Add MESSENGER_EXPOSED_METHODS somewhere in your controller file, e.g.:

core/packages/sample-controllers/src/sample-gas-prices-controller.ts

Line 91 in e5897c7

const MESSENGER_EXPOSED_METHODS = ['updateGasPrices'] as const;

Run yarn generate-method-action-types. This will generate a new file, AnalyticsController-method-action-types.ts.

Import AnalyticsControllerMethodActions from this file.

Add this type to AnalyticsControllerActions, e.g.:

core/packages/sample-controllers/src/sample-gas-prices-controller.ts

Line 106 in e5897c7

| SampleGasPricesControllerMethodActions;

Call this.messenger.registerMethodActionHandlers in your controller constructor:

core/packages/sample-controllers/src/sample-gas-prices-controller.ts

Line 256 in e5897c7

this.messenger.registerMethodActionHandlers(

mcmire · 2025-11-07T19:41:06Z

packages/ramps-controller/src/RampsController.test.ts

+  let mockConsoleError: jest.SpyInstance;
+
+  beforeEach(() => {
+    jest.clearAllMocks();


Mocks get automatically reset between tests in this repo, so you don't need this:

Suggested change

jest.clearAllMocks();

mcmire · 2025-11-07T19:42:02Z

packages/ramps-controller/src/RampsController.test.ts

+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mockFetch = jest.spyOn(global, 'fetch') as jest.MockedFunction<


We don't recommend mocking the fetch function directly. Nock is already set in this repo and that offers a more realistic way of mocking requests. You can see an example here:

core/packages/sample-controllers/src/sample-gas-prices-service/sample-gas-prices-service.test.ts

Line 29 in e144cc5

nock('https://api.example.com')

mcmire · 2025-11-07T19:42:21Z

packages/ramps-controller/src/RampsController.test.ts

+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+


Similar to comment above, this already happens by default.

Suggested change

afterEach(() => {

jest.restoreAllMocks();

});

cursor · 2025-11-07T22:01:13Z

packages/ramps-controller/src/RampsController.ts

+  metamaskEnvironment: SdkEnvironment.Staging,
+  context: Context.Browser,
+  region: null,
+};


Bug: Enforce Controller defaultState Guidelines (Bugbot Rules)

The defaultState constant violates controller guidelines. According to the repository's controller guidelines and PR feedback, default state must be wrapped in a function called getDefaultRampsControllerState() and exported. This prevents accidental mutations in tests and ensures a fresh object reference each time.

cursor · 2025-11-07T22:01:13Z

packages/ramps-controller/src/RampsController.ts

+  }: {
+    messenger: RampsControllerMessenger;
+    state: Partial<RampsControllerState>;
+  }) {


Bug: Flexible State Instantiation (Bugbot Rules)

The state constructor parameter is required but should be optional per controller guidelines. The type should be state?: Partial<RampsControllerState> with a default value of {}, allowing the controller to be instantiated without providing initial state.

pkowalski added 2 commits November 7, 2025 09:48

feat: foundations for ramp controller

aaded9c

feat: configuration

5c89786

pkowalski requested a review from a team as a code owner November 7, 2025 16:12

cursor bot reviewed Nov 7, 2025

View reviewed changes

tsconfig.build.json Outdated Show resolved Hide resolved

pkowalski added 3 commits November 7, 2025 10:14

chore: cleanup tsconfig

361c018

chore: fix tsconfig.build

9d8e4b0

feat: add remaining ramps controller files

3aec57f

pkowalski added the no-changelog label Nov 7, 2025

cursor bot reviewed Nov 7, 2025

View reviewed changes

mcmire reviewed Nov 7, 2025

View reviewed changes

fix: yarn.lock

9efa238

cursor bot reviewed Nov 7, 2025

View reviewed changes

Uh oh!

Ramps Controller Foundation #7091

Are you sure you want to change the base?

Ramps Controller Foundation #7091

Uh oh!

Conversation

pkowalski commented Nov 7, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

Current State:

Why we want to change things:

Changes introduced:

References

Checklist

Uh oh!

Uh oh!

cursor bot Nov 7, 2025

Choose a reason for hiding this comment

Bug: Unsafe geolocation: non-string API data breaks URLs

Uh oh!

socket-security bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcmire left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkowalski Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcmire Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot Nov 7, 2025

Choose a reason for hiding this comment

Bug: Enforce Controller defaultState Guidelines (Bugbot Rules)

Uh oh!

cursor bot Nov 7, 2025

Choose a reason for hiding this comment

Bug: Flexible State Instantiation (Bugbot Rules)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pkowalski commented Nov 7, 2025 •

edited by cursor bot

Loading

socket-security bot commented Nov 7, 2025 •

edited

Loading

pkowalski Nov 10, 2025 •

edited

Loading

mcmire Nov 7, 2025 •

edited

Loading

Bug: Enforce Controller `defaultState` Guidelines (Bugbot Rules)