Skip to content

feat: update getBalance method to require blockNumberOrTagOrHash parameter instead of allowing null (#3838) #3859

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions packages/relay/src/lib/eth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ export class EthImpl implements Eth {
* @rpcParamValidationRules Applies JSON-RPC parameter validation according to the API specification
*
* @param {string} account The account to get the balance from
* @param {string | null} blockNumberOrTagOrHash The block number or tag or hash to get the balance from
* @param {string} blockNumberOrTagOrHash The block number or tag or hash to get the balance from
* @param {RequestDetails} requestDetails The request details for logging and tracking
* @returns {Promise<string>} A promise that resolves to the balance of the account in hexadecimal format.
*/
Expand All @@ -714,11 +714,7 @@ export class EthImpl implements Eth {
@cache(CacheService.getInstance(CACHE_LEVEL.L1), {
skipParams: [{ index: '1', value: constants.NON_CACHABLE_BLOCK_PARAMS }],
})
async getBalance(
account: string,
blockNumberOrTagOrHash: string | null,
requestDetails: RequestDetails,
): Promise<string> {
async getBalance(account: string, blockNumberOrTagOrHash: string, requestDetails: RequestDetails): Promise<string> {
return this.accountService.getBalance(account, blockNumberOrTagOrHash, requestDetails);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ export class AccountService implements IAccountService {
* Current implementation does not yet utilize blockNumber
*
* @param {string} account The account to get the balance from
* @param {string | null} blockNumberOrTagOrHash The block number or tag or hash to get the balance from
* @param {string} blockNumberOrTagOrHash The block number or tag or hash to get the balance from

Choose a reason for hiding this comment

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

I thibk we should a test to enforce required. Somethin like cannot be bull, blockNumberOrTagOrHashIsRequired

* @param {RequestDetails} requestDetails The request details for logging and tracking
*/
public async getBalance(
account: string,
blockNumberOrTagOrHash: string | null,
blockNumberOrTagOrHash: string,
requestDetails: RequestDetails,
): Promise<string> {
const requestIdPrefix = requestDetails.formattedRequestId;
Expand Down Expand Up @@ -174,7 +174,7 @@ export class AccountService implements IAccountService {
* @param requestDetails
* @private
*/
private async extractBlockNumberAndTimestamp(blockNumberOrTagOrHash: string | null, requestDetails: RequestDetails) {
private async extractBlockNumberAndTimestamp(blockNumberOrTagOrHash: string, requestDetails: RequestDetails) {
let latestBlock: LatestBlockNumberTimestamp;
const latestBlockTolerance = 1;
let blockHashNumber, isHash;
Expand All @@ -192,7 +192,7 @@ export class AccountService implements IAccountService {
latestBlock = await this.blockNumberTimestamp(constants.ETH_GET_BALANCE, requestDetails);
}

if (blockNumberOrTagOrHash != null && blockNumberOrTagOrHash.length > 32) {
if (blockNumberOrTagOrHash.length > 32) {
isHash = true;
blockHashNumber = await this.mirrorNodeClient.getBlock(blockNumberOrTagOrHash, requestDetails);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,5 @@ export interface IAccountService {
requestDetails: RequestDetails,
) => Promise<string | JsonRpcError>;

getBalance: (
account: string,
blockNumberOrTagOrHash: string | null,
requestDetails: RequestDetails,
) => Promise<string>;
getBalance: (account: string, blockNumberOrTagOrHash: string, requestDetails: RequestDetails) => Promise<string>;
}
14 changes: 7 additions & 7 deletions packages/relay/tests/lib/eth/eth_getBalance.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,21 +59,21 @@ describe('@ethGetBalance using MirrorNode', async function () {
restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, JSON.stringify(MOCK_BLOCK_NUMBER_1000_RES));
restMock.onGet(`accounts/${CONTRACT_ADDRESS_1}?limit=100`).reply(200, JSON.stringify(MOCK_BALANCE_RES));

const resBalance = await ethImpl.getBalance(CONTRACT_ADDRESS_1, null, requestDetails);
const resBalance = await ethImpl.getBalance(CONTRACT_ADDRESS_1, 'latest', requestDetails);
expect(resBalance).to.equal(DEF_HEX_BALANCE);
});

it('should return balance for latest block from cache', async () => {
restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, JSON.stringify(MOCK_BLOCK_NUMBER_1000_RES));
restMock.onGet(`accounts/${CONTRACT_ADDRESS_1}?limit=100`).reply(200, JSON.stringify(MOCK_BALANCE_RES));

const resBalance = await ethImpl.getBalance(CONTRACT_ADDRESS_1, null, requestDetails);
const resBalance = await ethImpl.getBalance(CONTRACT_ADDRESS_1, 'latest', requestDetails);
expect(resBalance).to.equal(DEF_HEX_BALANCE);

// next call should use cache
restMock.onGet(`accounts/${CONTRACT_ADDRESS_1}?limit=100`).reply(404, {});

const resBalanceCached = await ethImpl.getBalance(CONTRACT_ADDRESS_1, null, requestDetails);
const resBalanceCached = await ethImpl.getBalance(CONTRACT_ADDRESS_1, 'latest', requestDetails);
expect(resBalanceCached).to.equal(resBalance);

// Third call should return new number using mirror node
Expand All @@ -91,7 +91,7 @@ describe('@ethGetBalance using MirrorNode', async function () {
// expire cache, instead of waiting for ttl we clear it to simulate expiry faster.
await cacheService.clear(requestDetails);

const resBalanceNew = await ethImpl.getBalance(CONTRACT_ADDRESS_1, null, requestDetails);
const resBalanceNew = await ethImpl.getBalance(CONTRACT_ADDRESS_1, 'latest', requestDetails);
expect(newBalanceHex).to.equal(resBalanceNew);
});

Expand Down Expand Up @@ -162,19 +162,19 @@ describe('@ethGetBalance using MirrorNode', async function () {
restMock.onGet(`contracts/${CONTRACT_ADDRESS_1}`).reply(200, JSON.stringify(null));
restMock.onGet(`accounts/${CONTRACT_ADDRESS_1}?limit=100`).reply(404, JSON.stringify(NOT_FOUND_RES));

const resBalance = await ethImpl.getBalance(CONTRACT_ADDRESS_1, null, requestDetails);
const resBalance = await ethImpl.getBalance(CONTRACT_ADDRESS_1, 'latest', requestDetails);
expect(resBalance).to.equal(constants.ZERO_HEX);
});

it('should return cached value for mirror nodes', async () => {
restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, JSON.stringify(MOCK_BLOCK_NUMBER_1000_RES));
restMock.onGet(`accounts/${CONTRACT_ADDRESS_1}?limit=100`).reply(200, JSON.stringify(MOCK_BALANCE_RES));

const resNoCache = await ethImpl.getBalance(CONTRACT_ADDRESS_1, null, requestDetails);
const resNoCache = await ethImpl.getBalance(CONTRACT_ADDRESS_1, 'latest', requestDetails);

restMock.onGet(`accounts/${CONTRACT_ADDRESS_1}?limit=100`).reply(404, NOT_FOUND_RES);

const resCached = await ethImpl.getBalance(CONTRACT_ADDRESS_1, null, requestDetails);
const resCached = await ethImpl.getBalance(CONTRACT_ADDRESS_1, 'latest', requestDetails);
expect(resNoCache).to.equal(DEF_HEX_BALANCE);
expect(resCached).to.equal(DEF_HEX_BALANCE);
});
Expand Down
Loading