From a6465a309532c7ebb37274660b9d52f4fd8fcbd5 Mon Sep 17 00:00:00 2001 From: Veetrag Jain Date: Wed, 9 Apr 2025 11:16:01 +0530 Subject: [PATCH] fix(abstract-eth): eliminate statics coin map dependency for tokens Ticket: WIN-5115 BREAKING CHANGE: eliminates statics coin map dependency for tokens(cannot use token name to fetch token details) --- .../src/abstractEthLikeNewCoins.ts | 34 +++++----------- modules/abstract-eth/src/ethLikeToken.ts | 4 +- modules/abstract-eth/src/lib/transaction.ts | 15 +------ .../abstract-eth/src/lib/transferBuilder.ts | 6 +-- .../transferBuilderERC1155.ts | 6 +-- .../transferBuilders/transferBuilderERC721.ts | 6 +-- modules/abstract-eth/src/lib/utils.ts | 40 +------------------ modules/abstract-eth/test/unit/coin.ts | 4 +- .../abstract-eth/test/unit/transferBuilder.ts | 12 ++---- 9 files changed, 30 insertions(+), 97 deletions(-) diff --git a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts index bec42ca05d..6d3ad5228f 100644 --- a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts +++ b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts @@ -68,7 +68,6 @@ import { ERC721TransferBuilder, getCommon, getProxyInitcode, - getToken, KeyPair as KeyPairLib, TransactionBuilder, TransferBuilder, @@ -1026,7 +1025,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin { txBuilder.from(params.txPrebuild.txHex); txBuilder .transfer() - .coin(this.staticsCoin?.name as string) + .coin(this.staticsCoin as Readonly) .key(new KeyPairLib({ prv: params.prv }).getKeys().prv!); if (params.walletVersion) { txBuilder.walletVersion(params.walletVersion); @@ -1302,7 +1301,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin { }); const transferBuilder = txBuilder.transfer() as TransferBuilder; transferBuilder - .coin(this.staticsCoin?.name as string) + .coin(this.staticsCoin as Readonly) .amount(recipients[0].amount) .contractSequenceId(sequenceId) .expirationTime(this.getDefaultExpireTime()) @@ -1330,7 +1329,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin { txBuilder .transfer() - .coin(this.staticsCoin?.name as string) + .coin(this.staticsCoin as Readonly) .key(new KeyPairLib({ prv: userKey }).getKeys().prv as string); txBuilder.sign({ key: backupSigningKey }); @@ -1373,7 +1372,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin { txBuilder.from(txHex); txBuilder .transfer() - .coin(this.staticsCoin?.name as string) + .coin(this.staticsCoin as Readonly) .key(userSigningKey); const tx = await txBuilder.build(); const res = await this.bitgo @@ -1524,14 +1523,14 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin { const transferBuilder = txBuilder.transfer() as TransferBuilder; if (!batcherContractAddress) { transferBuilder - .coin(this.staticsCoin?.name as string) + .coin(this.staticsCoin as Readonly) .amount(batchExecutionInfo.totalAmount) .contractSequenceId(sequenceId) .expirationTime(this.getDefaultExpireTime()) .to(recoveryDestination); } else { transferBuilder - .coin(this.staticsCoin?.name as string) + .coin(this.staticsCoin as Readonly) .amount(batchExecutionInfo.totalAmount) .contractSequenceId(sequenceId) .expirationTime(this.getDefaultExpireTime()) @@ -1691,26 +1690,13 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin { const transferBuilder = txBuilder.transfer() as TransferBuilder; - const network = this.getNetwork(); - const token = getToken( - params.tokenContractAddress as string, - network as EthLikeNetwork, - this.staticsCoin?.family as string - )?.name as string; - transferBuilder .amount(txAmount) .contractSequenceId(sequenceId) .expirationTime(this.getDefaultExpireTime()) - .to(params.recoveryDestination); - - if (token) { - transferBuilder.coin(token); - } else { - transferBuilder - .coin(this.staticsCoin?.name as string) - .tokenContractAddress(params.tokenContractAddress as string); - } + .to(params.recoveryDestination) + .coin(this.staticsCoin as Readonly) + .tokenContractAddress(params.tokenContractAddress as string); if (params.walletPassphrase) { txBuilder.transfer().key(userSigningKey); @@ -1751,7 +1737,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin { const response: OfflineVaultTxInfo = { txHex: tx.toBroadcastFormat(), userKey, - coin: token ? token : this.getChain(), + coin: this.getChain(), gasPrice: optionalDeps.ethUtil.bufferToInt(gasPrice).toFixed(), gasLimit, recipients: txInfo.recipients, diff --git a/modules/abstract-eth/src/ethLikeToken.ts b/modules/abstract-eth/src/ethLikeToken.ts index 1ced7ba321..64099ecac6 100644 --- a/modules/abstract-eth/src/ethLikeToken.ts +++ b/modules/abstract-eth/src/ethLikeToken.ts @@ -354,7 +354,7 @@ export class EthLikeToken extends AbstractEthLikeNewCoins { }); const transferBuilder = txBuilder.transfer() as EthLikeTransferBuilder; transferBuilder - .coin(this.tokenConfig.type) + .tokenContractAddress(this.tokenConfig.tokenContractAddress) .amount(recipients[0].amount) .contractSequenceId(sequenceId) .expirationTime(this.getDefaultExpireTime()) @@ -383,7 +383,7 @@ export class EthLikeToken extends AbstractEthLikeNewCoins { txBuilder .transfer() - .coin(this.tokenConfig.type) + .tokenContractAddress(this.tokenConfig.tokenContractAddress) .key(new KeyPairLib({ prv: userKey }).getKeys().prv as string); txBuilder.sign({ key: backupSigningKey }); diff --git a/modules/abstract-eth/src/lib/transaction.ts b/modules/abstract-eth/src/lib/transaction.ts index 8a347f7f96..44661da8d0 100644 --- a/modules/abstract-eth/src/lib/transaction.ts +++ b/modules/abstract-eth/src/lib/transaction.ts @@ -14,9 +14,7 @@ import { import { KeyPair } from './keyPair'; import { EthLikeTransactionData, TxData } from './iface'; import { EthTransactionData } from './types'; -import { classifyTransaction, decodeTransferData, getToken, hasSignature, toStringSig } from './utils'; - -const UNSUPPORTED_COIN_NAME = 'unsupported'; +import { classifyTransaction, decodeTransferData, hasSignature, toStringSig } from './utils'; export class Transaction extends BaseTransaction { protected _id: string; // The transaction id as seen in the blockchain @@ -98,25 +96,16 @@ export class Transaction extends BaseTransaction { this._type === TransactionType.SendERC721 || this._type === TransactionType.SendERC1155 ) { - const { to, amount, tokenContractAddress, signature } = decodeTransferData(txData.data); - let coinName: string; - if (tokenContractAddress) { - const token = getToken(tokenContractAddress, this._coinConfig.network, this._coinConfig.family); - coinName = token ? token.name : UNSUPPORTED_COIN_NAME; - } else { - coinName = this._coinConfig.name; - } + const { to, amount, signature } = decodeTransferData(txData.data); this.outputs.push({ address: to, value: amount, - coin: coinName, }); this.inputs.push({ address: txData.to!, // the sending wallet contract is the recipient of the outer transaction value: amount, - coin: coinName, }); this._signatures.push(signature); diff --git a/modules/abstract-eth/src/lib/transferBuilder.ts b/modules/abstract-eth/src/lib/transferBuilder.ts index f0fcddf8a0..5408e35b8e 100644 --- a/modules/abstract-eth/src/lib/transferBuilder.ts +++ b/modules/abstract-eth/src/lib/transferBuilder.ts @@ -2,7 +2,7 @@ import assert from 'assert'; import * as ethUtil from 'ethereumjs-util'; import EthereumAbi from 'ethereumjs-abi'; import BN from 'bn.js'; -import { coins, BaseCoin, ContractAddressDefinedToken, EthereumNetwork as EthLikeNetwork } from '@bitgo/statics'; +import { BaseCoin, ContractAddressDefinedToken, EthereumNetwork as EthLikeNetwork } from '@bitgo/statics'; import { BuildTransactionError, InvalidParameterValueError } from '@bitgo/sdk-core'; import { decodeTransferData, sendMultiSigData, sendMultiSigTokenData, isValidEthAddress, isValidAmount } from './utils'; import { defaultAbiCoder, keccak256 } from 'ethers/lib/utils'; @@ -42,8 +42,8 @@ export class TransferBuilder { * @param {string} coin - the native coin or ERC20 token to be set * @returns {TransferBuilder} the transfer builder instance modified */ - coin(coin: string): TransferBuilder { - this._coin = coins.get(coin); + coin(coin: Readonly): TransferBuilder { + this._coin = coin; if (this._coin instanceof ContractAddressDefinedToken) { this._tokenContractAddress = this._coin.contractAddress.toString(); } diff --git a/modules/abstract-eth/src/lib/transferBuilders/transferBuilderERC1155.ts b/modules/abstract-eth/src/lib/transferBuilders/transferBuilderERC1155.ts index b16c471edd..d9a1bd977d 100644 --- a/modules/abstract-eth/src/lib/transferBuilders/transferBuilderERC1155.ts +++ b/modules/abstract-eth/src/lib/transferBuilders/transferBuilderERC1155.ts @@ -10,7 +10,7 @@ import { ERC1155SafeTransferTypeMethodId, } from '../walletUtil'; import { BaseNFTTransferBuilder } from './baseNFTTransferBuilder'; -import { coins, EthereumNetwork as EthLikeNetwork } from '@bitgo/statics'; +import { BaseCoin, EthereumNetwork as EthLikeNetwork } from '@bitgo/statics'; export class ERC1155TransferBuilder extends BaseNFTTransferBuilder { private _tokenIds: string[]; @@ -28,8 +28,8 @@ export class ERC1155TransferBuilder extends BaseNFTTransferBuilder { } } - coin(coin: string): ERC1155TransferBuilder { - this._coin = coins.get(coin); + coin(coin: Readonly): ERC1155TransferBuilder { + this._coin = coin; this._nativeCoinOperationHashPrefix = (this._coin.network as EthLikeNetwork).nativeCoinOperationHashPrefix; return this; } diff --git a/modules/abstract-eth/src/lib/transferBuilders/transferBuilderERC721.ts b/modules/abstract-eth/src/lib/transferBuilders/transferBuilderERC721.ts index fec998cb8b..b2c9393ee5 100644 --- a/modules/abstract-eth/src/lib/transferBuilders/transferBuilderERC721.ts +++ b/modules/abstract-eth/src/lib/transferBuilders/transferBuilderERC721.ts @@ -5,7 +5,7 @@ import { ContractCall } from '../contractCall'; import { decodeERC721TransferData, isValidEthAddress, sendMultiSigData } from '../utils'; import { BaseNFTTransferBuilder } from './baseNFTTransferBuilder'; import { ERC721SafeTransferTypeMethodId, ERC721SafeTransferTypes } from '../walletUtil'; -import { coins, EthereumNetwork as EthLikeNetwork } from '@bitgo/statics'; +import { BaseCoin, EthereumNetwork as EthLikeNetwork } from '@bitgo/statics'; export class ERC721TransferBuilder extends BaseNFTTransferBuilder { private _tokenId: string; @@ -19,8 +19,8 @@ export class ERC721TransferBuilder extends BaseNFTTransferBuilder { } } - coin(coin: string): ERC721TransferBuilder { - this._coin = coins.get(coin); + coin(coin: Readonly): ERC721TransferBuilder { + this._coin = coin; this._nativeCoinOperationHashPrefix = (this._coin.network as EthLikeNetwork).nativeCoinOperationHashPrefix; return this; } diff --git a/modules/abstract-eth/src/lib/utils.ts b/modules/abstract-eth/src/lib/utils.ts index 8c01ce79ba..efae60910d 100644 --- a/modules/abstract-eth/src/lib/utils.ts +++ b/modules/abstract-eth/src/lib/utils.ts @@ -1,6 +1,5 @@ import { Buffer } from 'buffer'; import request from 'superagent'; -import assert from 'assert'; import { addHexPrefix, bufferToHex, @@ -13,7 +12,7 @@ import { generateAddress2, padToEven, } from 'ethereumjs-util'; -import { BaseCoin, BaseNetwork, coins, ContractAddressDefinedToken, EthereumNetwork } from '@bitgo/statics'; +import { coins, EthereumNetwork } from '@bitgo/statics'; import EthereumAbi from 'ethereumjs-abi'; import EthereumCommon from '@ethereumjs/common'; import BN from 'bn.js'; @@ -657,43 +656,6 @@ export function getBufferedByteCode(methodId: string, rawData: string): Buffer { return Buffer.from(splitBytecode[1], 'hex'); } -/** - * Get the statics coin object matching a given contract address if it exists - * - * @param tokenContractAddress The contract address to match against - * @param network - the coin network - * @param family - the coin family - * @returns statics BaseCoin object for the matching token - */ -export function getToken( - tokenContractAddress: string, - network: BaseNetwork, - family: string -): Readonly | undefined { - // filter the coins array to find the token with the matching contract address, network and coin family - // coin family is needed to avoid causing issues when a token has same contract address on two different chains - const tokens = coins.filter((coin) => { - if (coin instanceof ContractAddressDefinedToken) { - return ( - coin.network.type === network.type && - coin.family === family && - coin.contractAddress.toLowerCase() === tokenContractAddress.toLowerCase() - ); - } - return false; - }); - - // if length of tokens is 1, return the first, else return undefined - // Can't directly index into tokens, or call `length`, so we use map to get an array - const tokensArray = tokens.map((token) => token); - if (tokensArray.length >= 1) { - // there should never be two tokens with the same contract address, so we assert that here - assert(tokensArray.length === 1); - return tokensArray[0]; - } - return undefined; -} - /** * Returns the create wallet method calling data for v1 wallets * diff --git a/modules/abstract-eth/test/unit/coin.ts b/modules/abstract-eth/test/unit/coin.ts index 06b9010246..318e1e1c62 100644 --- a/modules/abstract-eth/test/unit/coin.ts +++ b/modules/abstract-eth/test/unit/coin.ts @@ -11,6 +11,7 @@ import { TransferBuilder, TransactionBuilder, } from '../../src'; +import { coins } from '@bitgo/statics'; nock.enableNetConnect(); @@ -173,7 +174,8 @@ export function runSignTransactionTests(coinName: string, builder: TransactionBu builder.type(TransactionType.Send); builder.contract(account_1.address); const transferBuilder = builder.transfer() as TransferBuilder; - transferBuilder.coin(coinTest).amount('1').to(account_2.address).expirationTime(10000).contractSequenceId(1); + const staticsCoin = coins.get(coinTest); + transferBuilder.coin(staticsCoin).amount('1').to(account_2.address).expirationTime(10000).contractSequenceId(1); const unsignedTx = await builder.build(); const unsignedTxForBroadcasting = unsignedTx.toBroadcastFormat(); diff --git a/modules/abstract-eth/test/unit/transferBuilder.ts b/modules/abstract-eth/test/unit/transferBuilder.ts index 53dd3317a8..d060881ba4 100644 --- a/modules/abstract-eth/test/unit/transferBuilder.ts +++ b/modules/abstract-eth/test/unit/transferBuilder.ts @@ -12,24 +12,18 @@ describe('Eth send multi sig builder', function () { const ethLikeCoins = ['hteth', 'tarbeth', 'topeth', 'zketh']; describe('should fail', () => { - it('should fail if a coin does not exists in @bitgo/statics', () => { - should(() => { - new TransferBuilder().coin('inexistentcoin'); - }).throw(); - }); - ethLikeCoins.forEach((coin) => { it('should fail with an invalid key', () => { - const staticsCoin = coins.get(coin) as unknown as EthLikeNetwork; + const staticsCoin = coins.get(coin); const builder = new TransferBuilder() - .coin(coin) + .coin(staticsCoin) .expirationTime(1590078260) .amount(amount) .to(toAddress) .contractSequenceId(2) .key('invalidkey'); should(() => { - builder.signAndBuild(`${staticsCoin.chainId}`); + builder.signAndBuild(`${(staticsCoin as unknown as EthLikeNetwork).chainId}`); }).throw('private key length is invalid'); }); });