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

Refactoring Clients usage #20

Merged
merged 11 commits into from
May 21, 2024
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
"@types/secp256k1": "^4.0.6",
"@typescript-eslint/eslint-plugin": "^7.8.0",
"@typescript-eslint/parser": "^7.8.0",
"axios-mock-adapter": "^1.22.0",
"eslint": "^8.57.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-jsdoc": "^48.2.5",
Expand Down
16 changes: 5 additions & 11 deletions src/coinbase/address.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,28 @@
import { Address as AddressModel } from "../client";
import { Balance } from "./balance";
import { BalanceMap } from "./balance_map";
import { Coinbase } from "./coinbase";
import { InternalError } from "./errors";
import { FaucetTransaction } from "./faucet_transaction";
import { AddressAPIClient } from "./types";
import { Decimal } from "decimal.js";

/**
* A representation of a blockchain address, which is a user-controlled account on a network.
*/
export class Address {
private model: AddressModel;
private client: AddressAPIClient;

/**
* Initializes a new Address instance.
*
* @param {AddressModel} model - The address model data.
* @param {AddressAPIClient} client - The API client to interact with address-related endpoints.
* @throws {InternalError} If the model or client is empty.
*/
constructor(model: AddressModel, client: AddressAPIClient) {
constructor(model: AddressModel) {
if (!model) {
throw new InternalError("Address model cannot be empty");
}
if (!client) {
throw new InternalError("Address client cannot be empty");
}
this.model = model;
this.client = client;
}

/**
Expand All @@ -40,7 +34,7 @@ export class Address {
* @throws {Error} If the request fails.
*/
async faucet(): Promise<FaucetTransaction> {
const response = await this.client.requestFaucetFunds(
const response = await Coinbase.apiClients.address!.requestFaucetFunds(
this.model.wallet_id,
this.model.address_id,
);
Expand Down Expand Up @@ -71,7 +65,7 @@ export class Address {
* @returns {BalanceMap} - The map from asset ID to balance.
*/
async listBalances(): Promise<BalanceMap> {
const response = await this.client.listAddressBalances(
const response = await Coinbase.apiClients.address!.listAddressBalances(
this.model.wallet_id,
this.model.address_id,
);
Expand All @@ -86,7 +80,7 @@ export class Address {
* @returns {Decimal} The balance of the asset.
*/
async getBalance(assetId: string): Promise<Decimal> {
const response = await this.client.getAddressBalance(
const response = await Coinbase.apiClients.address!.getAddressBalance(
this.model.wallet_id,
this.model.address_id,
assetId,
Expand Down
18 changes: 10 additions & 8 deletions src/coinbase/coinbase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class Coinbase {
Weth: "weth",
};

apiClients: ApiClients = {};
static apiClients: ApiClients = {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Coinbase.apiClients is now globally accessible, so there's no need to pass clients to the Model class anymore.


/**
* Represents the number of Wei per Ether.
Expand Down Expand Up @@ -85,11 +85,13 @@ export class Coinbase {
response => logApiResponse(response, debugging),
);

this.apiClients.user = UsersApiFactory(config, BASE_PATH, axiosInstance);
this.apiClients.wallet = WalletsApiFactory(config, BASE_PATH, axiosInstance);
this.apiClients.address = AddressesApiFactory(config, BASE_PATH, axiosInstance);
this.apiClients.transfer = TransfersApiFactory(config, BASE_PATH, axiosInstance);
this.apiClients.baseSepoliaProvider = new ethers.JsonRpcProvider("https://sepolia.base.org");
Coinbase.apiClients.user = UsersApiFactory(config, BASE_PATH, axiosInstance);
Coinbase.apiClients.wallet = WalletsApiFactory(config, BASE_PATH, axiosInstance);
Coinbase.apiClients.address = AddressesApiFactory(config, BASE_PATH, axiosInstance);
Coinbase.apiClients.transfer = TransfersApiFactory(config, BASE_PATH, axiosInstance);
Coinbase.apiClients.baseSepoliaProvider = new ethers.JsonRpcProvider(
"https://sepolia.base.org",
);
}

/**
Expand Down Expand Up @@ -137,7 +139,7 @@ export class Coinbase {
* @throws {APIError} If the request fails.
*/
async getDefaultUser(): Promise<User> {
const userResponse = await this.apiClients.user!.getCurrentUser();
return new User(userResponse.data as UserModel, this.apiClients);
const userResponse = await Coinbase.apiClients.user!.getCurrentUser();
return new User(userResponse.data as UserModel);
}
}
189 changes: 84 additions & 105 deletions src/coinbase/tests/address_test.ts
Original file line number Diff line number Diff line change
@@ -1,91 +1,48 @@
import { ethers } from "ethers";
import {
AddressBalanceList,
AddressesApiFactory,
Address as AddressModel,
Balance as BalanceModel,
} from "../../client";
import { Address } from "./../address";
import { FaucetTransaction } from "./../faucet_transaction";

import MockAdapter from "axios-mock-adapter";
import { randomUUID } from "crypto";
import Decimal from "decimal.js";
import { APIError, FaucetLimitReachedError } from "../api_error";
import { Coinbase } from "../coinbase";
import { InternalError } from "../errors";
import { createAxiosMock } from "./utils";
import Decimal from "decimal.js";

// newAddressModel creates a new AddressModel with a random wallet ID and a random Ethereum address.
export const newAddressModel = (walletId: string): AddressModel => {
const ethAddress = ethers.Wallet.createRandom();

return {
address_id: ethAddress.address,
network_id: Coinbase.networkList.BaseSepolia,
public_key: ethAddress.publicKey,
wallet_id: walletId,
};
};

const VALID_ADDRESS_MODEL = newAddressModel(randomUUID());

const VALID_BALANCE_MODEL: BalanceModel = {
amount: "1000000000000000000",
asset: {
asset_id: Coinbase.assetList.Eth,
network_id: Coinbase.networkList.BaseSepolia,
},
};

const VALID_ADDRESS_BALANCE_LIST: AddressBalanceList = {
data: [
{
amount: "1000000000000000000",
asset: {
asset_id: Coinbase.assetList.Eth,
network_id: Coinbase.networkList.BaseSepolia,
decimals: 18,
},
},
{
amount: "5000000000",
asset: {
asset_id: "usdc",
network_id: Coinbase.networkList.BaseSepolia,
decimals: 6,
},
},
{
amount: "3000000000000000000",
asset: {
asset_id: "weth",
network_id: Coinbase.networkList.BaseSepolia,
decimals: 6,
},
},
],
has_more: false,
next_page: "",
total_count: 3,
};
import {
VALID_ADDRESS_BALANCE_LIST,
VALID_ADDRESS_MODEL,
addressesApiMock,
generateRandomHash,
mockFn,
mockReturnRejectedValue,
} from "./utils";

// Test suite for Address class
describe("Address", () => {
const [axiosInstance, configuration, BASE_PATH] = createAxiosMock();
const client = AddressesApiFactory(configuration, BASE_PATH, axiosInstance);
let address: Address, axiosMock;
const transactionHash = generateRandomHash();
let address: Address, balanceModel;

beforeAll(() => {
axiosMock = new MockAdapter(axiosInstance);
Coinbase.apiClients.address = addressesApiMock;
Coinbase.apiClients.address!.getAddressBalance = mockFn(request => {
const { asset_id } = request;
balanceModel = {
amount: "1000000000000000000",
asset: {
asset_id,
network_id: Coinbase.networkList.BaseSepolia,
},
};
return { data: balanceModel };
});
Coinbase.apiClients.address!.listAddressBalances = mockFn(() => {
return { data: VALID_ADDRESS_BALANCE_LIST };
});
Coinbase.apiClients.address!.requestFaucetFunds = mockFn(() => {
return { data: { transaction_hash: transactionHash } };
});
});
Comment on lines +24 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

why are these in the beforeAll block? It might be better for us to place these closer to the test that they are used in. It is unclear which test they are used in with this setup. We can use nested describe blocks to provide further context for the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I moved this beforeAll setup under the .create section. I used beforeAll because the mocks don't need to be updated after each test.


beforeEach(() => {
address = new Address(VALID_ADDRESS_MODEL, client);
});

afterEach(() => {
axiosMock.reset();
address = new Address(VALID_ADDRESS_MODEL);
jest.clearAllMocks();
});

it("should initialize a new Address", () => {
Expand All @@ -101,85 +58,107 @@ describe("Address", () => {
});

it("should return the correct list of balances", async () => {
axiosMock.onGet().reply(200, VALID_ADDRESS_BALANCE_LIST);
const balances = await address.listBalances();
expect(balances.get(Coinbase.assetList.Eth)).toEqual(new Decimal(1));
expect(balances.get("usdc")).toEqual(new Decimal(5000));
expect(balances.get("weth")).toEqual(new Decimal(3));
expect(Coinbase.apiClients.address!.listAddressBalances).toHaveBeenCalledWith(
address.getWalletId(),
address.getId(),
);
expect(Coinbase.apiClients.address!.listAddressBalances).toHaveBeenCalledTimes(1);
});

it("should return the correct ETH balance", async () => {
axiosMock.onGet().reply(200, VALID_BALANCE_MODEL);
const ethBalance = await address.getBalance(Coinbase.assetList.Eth);
expect(ethBalance).toBeInstanceOf(Decimal);
expect(ethBalance).toEqual(new Decimal(1));
expect(Coinbase.apiClients.address!.getAddressBalance).toHaveBeenCalledWith(
address.getWalletId(),
address.getId(),
Coinbase.assetList.Eth,
);
expect(Coinbase.apiClients.address!.getAddressBalance).toHaveBeenCalledTimes(1);
});

it("should return the correct Gwei balance", async () => {
axiosMock.onGet().reply(200, VALID_BALANCE_MODEL);
const ethBalance = await address.getBalance("gwei");
const assetId = "gwei";
const ethBalance = await address.getBalance(assetId);
expect(ethBalance).toBeInstanceOf(Decimal);
expect(ethBalance).toEqual(new Decimal(1000000000));
expect(Coinbase.apiClients.address!.getAddressBalance).toHaveBeenCalledWith(
address.getWalletId(),
address.getId(),
assetId,
);
expect(Coinbase.apiClients.address!.getAddressBalance).toHaveBeenCalledTimes(1);
});

it("should return the correct Wei balance", async () => {
axiosMock.onGet().reply(200, VALID_BALANCE_MODEL);
const ethBalance = await address.getBalance("wei");
const assetId = "wei";
const ethBalance = await address.getBalance(assetId);
expect(ethBalance).toBeInstanceOf(Decimal);
expect(ethBalance).toEqual(new Decimal(1000000000000000000));
expect(Coinbase.apiClients.address!.getAddressBalance).toHaveBeenCalledWith(
address.getWalletId(),
address.getId(),
assetId,
);
expect(Coinbase.apiClients.address!.getAddressBalance).toHaveBeenCalledTimes(1);
});

it("should return an error for an unsupported asset", async () => {
axiosMock.onGet().reply(404, null);
try {
await address.getBalance("unsupported-asset");
fail("Expect 404 to be thrown");
} catch (error) {
expect(error).toBeInstanceOf(APIError);
}
const getAddressBalance = jest.fn().mockRejectedValue(new APIError(""));
const assetId = "unsupported-asset";
Copy link
Contributor

Choose a reason for hiding this comment

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

Mixed usage of jest.fn() and mockFn from ./utils.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! I'm updating.

Coinbase.apiClients.address!.getAddressBalance = getAddressBalance;
await expect(address.getBalance(assetId)).rejects.toThrow(APIError);
expect(getAddressBalance).toHaveBeenCalledWith(address.getWalletId(), address.getId(), assetId);
expect(getAddressBalance).toHaveBeenCalledTimes(1);
});
Comment on lines +115 to +116
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To create tests similar to the ones in Ruby, I've added toHaveBeenCalledWith and toHaveBeenCalledTimes assertions.

Reference: https://github.com/coinbase/coinbase-sdk-ruby/blob/master/spec/unit/coinbase/address_spec.rb#L94


it("should return the wallet ID", () => {
expect(address.getWalletId()).toBe(VALID_ADDRESS_MODEL.wallet_id);
});

it("should throw an InternalError when model is not provided", () => {
expect(() => new Address(null!, client)).toThrow(`Address model cannot be empty`);
});

it("should throw an InternalError when client is not provided", () => {
expect(() => new Address(VALID_ADDRESS_MODEL, null!)).toThrow(`Address client cannot be empty`);
expect(() => new Address(null!)).toThrow(`Address model cannot be empty`);
});

it("should request funds from the faucet and returns the faucet transaction", async () => {
const transactionHash = "0xdeadbeef";
axiosMock.onPost().reply(200, {
transaction_hash: transactionHash,
});
const faucetTransaction = await address.faucet();
expect(faucetTransaction).toBeInstanceOf(FaucetTransaction);
expect(faucetTransaction.getTransactionHash()).toBe(transactionHash);
expect(Coinbase.apiClients.address!.requestFaucetFunds).toHaveBeenCalledWith(
address.getWalletId(),
address.getId(),
);
expect(Coinbase.apiClients.address!.requestFaucetFunds).toHaveBeenCalledTimes(1);
});

it("should throw an APIError when the request is unsuccesful", async () => {
axiosMock.onPost().reply(400);
Coinbase.apiClients.address!.requestFaucetFunds = mockReturnRejectedValue(new APIError(""));
await expect(address.faucet()).rejects.toThrow(APIError);
expect(Coinbase.apiClients.address!.requestFaucetFunds).toHaveBeenCalledWith(
address.getWalletId(),
address.getId(),
);
expect(Coinbase.apiClients.address!.requestFaucetFunds).toHaveBeenCalledTimes(1);
});

it("should throw a FaucetLimitReachedError when the faucet limit is reached", async () => {
axiosMock.onPost().reply(429, {
code: "faucet_limit_reached",
message: "Faucet limit reached",
});
Coinbase.apiClients.address!.requestFaucetFunds = mockReturnRejectedValue(
new FaucetLimitReachedError(""),
);
await expect(address.faucet()).rejects.toThrow(FaucetLimitReachedError);
expect(Coinbase.apiClients.address!.requestFaucetFunds).toHaveBeenCalledTimes(1);
});

it("should throw an InternalError when the request fails unexpectedly", async () => {
axiosMock.onPost().reply(500, {
code: "internal",
message: "unexpected error occurred while requesting faucet funds",
});
Coinbase.apiClients.address!.requestFaucetFunds = mockReturnRejectedValue(
new InternalError(""),
);
await expect(address.faucet()).rejects.toThrow(InternalError);
expect(Coinbase.apiClients.address!.requestFaucetFunds).toHaveBeenCalledTimes(1);
});

it("should return the correct string representation", () => {
Expand Down
Loading
Loading