chore(avaxc) : wrw support for evm ccr #4944

nrjsuthar · 2024-09-25T11:43:54Z

chore(avaxc) : wrw support for evm ccr

TICKET: COIN-1708

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts

kamleshmugdiya · 2024-09-25T15:34:26Z

tests are failing

kamleshmugdiya

this defeats the purpose of inheritance and code reusability.

kamleshmugdiya · 2024-09-25T15:41:16Z

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts

+    return userGasPrice;
+  }
+
+  async recoveryBlockchainExplorerQuery(query: Record<string, any>): Promise<any> {


kamleshmugdiya · 2024-09-25T15:55:46Z

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts

+  setGasLimit(userGasLimit?: number): number {
+    if (!userGasLimit) {
+      return ethGasConfigs.defaultGasLimit;
+    }
+    const gasLimitMax = ethGasConfigs.maximumGasLimit;
+    const gasLimitMin = ethGasConfigs.minimumGasLimit;
+    if (userGasLimit < gasLimitMin || userGasLimit > gasLimitMax) {
+      throw new Error(`Gas limit must be between ${gasLimitMin} and ${gasLimitMax}`);
+    }
+    return userGasLimit;
+  }


why is this required?
the implementation of setGasLimit in AbstractEthLikeNewCoins is the same.

kamleshmugdiya · 2024-09-25T15:55:57Z

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts

+  setGasPrice(userGasPrice?: number): number {
+    if (!userGasPrice) {
+      return ethGasConfigs.defaultGasPrice;
+    }
+
+    const gasPriceMax = ethGasConfigs.maximumGasPrice;
+    const gasPriceMin = ethGasConfigs.minimumGasPrice;
+    if (userGasPrice < gasPriceMin || userGasPrice > gasPriceMax) {
+      throw new Error(`Gas price must be between ${gasPriceMin} and ${gasPriceMax}`);
+    }
+    return userGasPrice;
+  }


why is this required?
the implementation of setGasPrice in AbstractEthLikeNewCoins is the same.

kamleshmugdiya · 2024-09-25T15:57:45Z

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts

+  async recoveryBlockchainExplorerQuery(query: Record<string, any>): Promise<any> {
+    const env = this.bitgo.getEnv();
+    const response = await request.post(common.Environments[env].avaxcNetworkBaseUrl + '/ext/bc/C/rpc').send(query);
+    if (!response.ok) {
+      throw new Error('could not reach avax.network');
+    }
+
+    if (response.body.status === '0' && response.body.message === 'NOTOK') {
+      throw new Error('avax.network rate limit reached');
+    }
+    return response.body;
+  }


why is this required?
the implementation of recoveryBlockchainExplorerQuery in AbstractEthLikeNewCoins is the same.

kamleshmugdiya · 2024-09-25T15:58:43Z

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts

+  async getAddressNonce(address: string): Promise<number> {
+    // Get nonce for backup key (should be 0)
+    const result = await this.recoveryBlockchainExplorerQuery({
+      jsonrpc: '2.0',
+      method: 'eth_getTransactionCount',
+      params: [address, 'latest'],
+      id: 1,
+    });
+    if (!result || isNaN(result.result)) {
+      throw new Error('Unable to find next nonce from avax.network, got: ' + JSON.stringify(result));
+    }
+    const nonceHex = result.result;
+    return new optionalDeps.ethUtil.BN(nonceHex.slice(2), 16).toNumber();
+  }


kamleshmugdiya · 2024-09-25T15:59:12Z

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts

+  async queryAddressBalance(address: string): Promise<BN> {
+    const result = await this.recoveryBlockchainExplorerQuery({
+      jsonrpc: '2.0',
+      method: 'eth_getBalance',
+      params: [address, 'latest'],
+      id: 1,
+    });
+    // throw if the result does not exist or the result is not a valid number
+    if (!result || !result.result || isNaN(result.result)) {
+      throw new Error(`Could not obtain address balance for ${address} from avax.network, got: ${result.result}`);
+    }
+    const nativeBalanceHex = result.result;
+    return new optionalDeps.ethUtil.BN(nativeBalanceHex.slice(2), 16);
+  }


kamleshmugdiya · 2024-09-25T15:59:45Z

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts

+  async querySequenceId(address: string): Promise<number> {
+    // Get sequence ID using contract call
+    const sequenceIdMethodSignature = optionalDeps.ethAbi.methodID('getNextSequenceId', []);
+    const sequenceIdArgs = optionalDeps.ethAbi.rawEncode([], []);
+    const sequenceIdData = Buffer.concat([sequenceIdMethodSignature, sequenceIdArgs]).toString('hex');
+    const sequenceIdDataHex = optionalDeps.ethUtil.addHexPrefix(sequenceIdData);
+    const result = await this.recoveryBlockchainExplorerQuery({
+      jsonrpc: '2.0',
+      method: 'eth_call',
+      params: [{ to: address, data: sequenceIdDataHex }, 'latest'],
+      id: 1,
+    });
+    if (!result || !result.result) {
+      throw new Error('Could not obtain sequence ID from avax.network, got: ' + result.result);
+    }
+    const sequenceIdHex = result.result;
+    return new optionalDeps.ethUtil.BN(sequenceIdHex.slice(2), 16).toNumber();
+  }


kamleshmugdiya · 2024-09-25T16:01:36Z

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts

+  async getGasPriceFromExternalAPI(): Promise<BN> {
+    try {
+      // COIN -1708 : hardcoded for half signing
+      const gasPrice = new BN(250000);
+      console.log(` Got hardcoded gas price: ${gasPrice}`);
+      return gasPrice;
+    } catch (e) {
+      throw new Error('Failed to get gas price');
+    }
+  }


nrjsuthar · 2024-09-25T16:45:24Z

@kamleshmugdiya
This PR #4925 had the change you mentioned, where the relationship exists. The changes in the current PR were because of the Gas and Limit, wherein all the parent methods are copied. The ideal change would be to extend avaxc class with abstractEthLikeNewCoin (mentioned PR), but removed as no one was sure of the change.

nrjsuthar · 2024-09-25T16:46:32Z

Also, the gas price and limit from the explorer were not required here for the half sign.

nrjsuthar · 2024-09-25T16:48:28Z

tests are failing

on it

kamleshmugdiya · 2024-09-25T17:11:20Z

@kamleshmugdiya This PR #4925 had the change you mentioned, where the relationship exists. The changes in the current PR were because of the Gas and Limit, wherein all the parent methods are copied. The ideal change would be to extend avaxc class with abstractEthLikeNewCoin (mentioned PR), but removed as no one was sure of the change.

I don't understand this, if the function has the same impl in AbstractEthLikeNewCoins, what is the need to override with the same code in the new class?

nrjsuthar · 2024-09-25T17:17:26Z

@kamleshmugdiya This PR #4925 had the change you mentioned, where the relationship exists. The changes in the current PR were because of the Gas and Limit, wherein all the parent methods are copied. The ideal change would be to extend avaxc class with abstractEthLikeNewCoin (mentioned PR), but removed as no one was sure of the change.

I don't understand this, if the function has the same impl in AbstractEthLikeNewCoins, what is the need to override with the same code in the new class?

This was based on testing, for each of the issues, the method has been copied.

Example:

A -> B -> C is the method flow and A has to be changed, now i we need to change the method A over here, we will need to override all its parents, the sub method will not be able to be called.

kamleshmugdiya · 2024-09-25T18:27:38Z

@kamleshmugdiya This PR #4925 had the change you mentioned, where the relationship exists. The changes in the current PR were because of the Gas and Limit, wherein all the parent methods are copied. The ideal change would be to extend avaxc class with abstractEthLikeNewCoin (mentioned PR), but removed as no one was sure of the change.

I don't understand this, if the function has the same impl in AbstractEthLikeNewCoins, what is the need to override with the same code in the new class?

This was based on testing, for each of the issues, the method has been copied.

Example:

A -> B -> C is the method flow and A has to be changed, now i we need to change the method A over here, we will need to override all its parents, the sub method will not be able to be called.

what testing?
and the methods that I pointed out, do they have any difference in implementation vs the AbstractEthLikeNewCoins?
they all looked the same to me.

modules/sdk-coin-avaxc/src/avaxc.ts

TICKET: COIN-1708

kamleshmugdiya · 2024-09-26T07:58:25Z

pipeline failing again

nrjsuthar requested a review from a team as a code owner September 25, 2024 11:43

nrjsuthar requested review from venkateshv1266, mullapudipruthvik and kamleshmugdiya September 25, 2024 11:46

github-advanced-security bot found potential problems Sep 25, 2024

View reviewed changes

modules/sdk-coin-avaxc/src/EthLikeAvaxc.ts Fixed Show fixed Hide fixed

nrjsuthar force-pushed the feature/COIN-1708-new branch 2 times, most recently from 773895f to c116a8e Compare September 25, 2024 12:41

kamleshmugdiya requested changes Sep 25, 2024

View reviewed changes

nrjsuthar force-pushed the feature/COIN-1708-new branch 2 times, most recently from 69b11e7 to bf631db Compare September 25, 2024 17:04

nrjsuthar force-pushed the feature/COIN-1708-new branch from bf631db to 28a78e4 Compare September 25, 2024 17:13

nrjsuthar force-pushed the feature/COIN-1708-new branch from 28a78e4 to c48ecda Compare September 25, 2024 17:29

kamleshmugdiya reviewed Sep 26, 2024

View reviewed changes

modules/sdk-coin-avaxc/src/avaxc.ts Show resolved Hide resolved

feat(sdk-coin-avaxc): add avaxc support for evm ccr

6818a6d

TICKET: COIN-1708

nrjsuthar force-pushed the feature/COIN-1708-new branch from c48ecda to 6818a6d Compare September 26, 2024 07:51

nrjsuthar closed this Sep 26, 2024

nrjsuthar deleted the feature/COIN-1708-new branch September 26, 2024 17:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(avaxc) : wrw support for evm ccr #4944

chore(avaxc) : wrw support for evm ccr #4944

nrjsuthar commented Sep 25, 2024

kamleshmugdiya commented Sep 25, 2024

kamleshmugdiya left a comment

kamleshmugdiya Sep 25, 2024

nrjsuthar Sep 25, 2024

kamleshmugdiya Sep 25, 2024

kamleshmugdiya Sep 25, 2024

kamleshmugdiya Sep 25, 2024

kamleshmugdiya Sep 25, 2024

kamleshmugdiya Sep 25, 2024

kamleshmugdiya Sep 25, 2024

kamleshmugdiya Sep 25, 2024

nrjsuthar commented Sep 25, 2024

nrjsuthar commented Sep 25, 2024

nrjsuthar commented Sep 25, 2024

kamleshmugdiya commented Sep 25, 2024

nrjsuthar commented Sep 25, 2024

kamleshmugdiya commented Sep 25, 2024

kamleshmugdiya commented Sep 26, 2024

chore(avaxc) : wrw support for evm ccr #4944

chore(avaxc) : wrw support for evm ccr #4944

Conversation

nrjsuthar commented Sep 25, 2024

kamleshmugdiya commented Sep 25, 2024

kamleshmugdiya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nrjsuthar commented Sep 25, 2024

nrjsuthar commented Sep 25, 2024

nrjsuthar commented Sep 25, 2024

kamleshmugdiya commented Sep 25, 2024

nrjsuthar commented Sep 25, 2024

kamleshmugdiya commented Sep 25, 2024

kamleshmugdiya commented Sep 26, 2024