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

Reduce instances of cached contract addresses #547

Merged
merged 5 commits into from
Mar 29, 2019
Merged
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
27 changes: 7 additions & 20 deletions app/util/transactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { rawEncode, rawDecode } from 'ethereumjs-abi';
import Engine from '../core/Engine';
import { strings } from '../../locales/i18n';
import contractMap from 'eth-contract-metadata';
import { isSmartContractCode } from 'gaba/util';

export const TOKEN_METHOD_TRANSFER = 'transfer';
export const TOKEN_METHOD_APPROVE = 'approve';
Expand All @@ -29,14 +30,6 @@ class ActionKeys {
static cache = {};
}

/**
* Utility class with the single responsibility
* of caching SmartContractAddresses
*/
class SmartContractAddresses {
static cache = {};
}

/**
* Utility class with the single responsibility
* of caching CollectibleAddresses
Expand Down Expand Up @@ -143,22 +136,15 @@ export function getMethodData(data) {
* @returns {boolean} - Wether the given address is a contract
*/
export async function isSmartContractAddress(address) {
const cache = SmartContractAddresses.cache[address];
if (cache) {
return Promise.resolve(cache);
}

address = toChecksumAddress(address);
// If in contract map we don't need to cache it
if (contractMap[address]) {
SmartContractAddresses.cache[address] = true;
return Promise.resolve(true);
}

const { TransactionController } = Engine.context;
const code = address ? await TransactionController.query('getCode', [address]) : undefined;
// Geth will return '0x', and ganache-core v2.2.1 will return '0x0'
const codeIsEmpty = !code || code === '0x' || code === '0x0';
SmartContractAddresses.cache[address] = !codeIsEmpty;
return !codeIsEmpty;
const isSmartContract = isSmartContractCode(code);
return isSmartContract;
}

/**
Expand Down Expand Up @@ -219,7 +205,8 @@ export async function getTransactionActionKey(transaction) {
return ret;
}
}
const toSmartContract = await isSmartContractAddress(to);
const toSmartContract =
transaction.toSmartContract !== undefined ? transaction.toSmartContract : await isSmartContractAddress(to);
if (toSmartContract) {
// There is no data or unknown method data, if is smart contract
ret = SMART_CONTRACT_INTERACTION_ACTION_KEY;
Expand Down
28 changes: 24 additions & 4 deletions app/util/transactions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,20 @@ describe('Transactions utils :: isSmartContractAddress', () => {
await isSmartContractAddress('0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359');
expect(stub).not.toBeCalled();
});

it('isSmartContractAddress should call query if not cached', async () => {
Engine.context = MOCK_ENGINE.context;
const stub = spyOn(Engine.context.TransactionController, 'query');
await isSmartContractAddress('0x1', '0x123');
expect(stub).toBeCalled();
});

it('isSmartContractAddress should call query if only address was provided', async () => {
Engine.context = MOCK_ENGINE.context;
const stub = spyOn(Engine.context.TransactionController, 'query');
await isSmartContractAddress('0x1');
expect(stub).toBeCalled();
});
});

describe('Transactions utils :: getTransactionActionKey', () => {
Expand All @@ -109,21 +123,27 @@ describe('Transactions utils :: getTransactionActionKey', () => {
const randomData = '0x987654321';
const transferFromData = '0x23b872dd0000000000000000000000000000';
it('getTransactionActionKey send ether', async () => {
const get = await getTransactionActionKey({ transactionHash: '0x1', transaction: {} });
const get = await getTransactionActionKey({ transactionHash: '0x1', transaction: { to: '0x0' } });
expect(get).toEqual(SEND_ETHER_ACTION_KEY);
});

it('getTransactionActionKey send ether with empty data', async () => {
const get = await getTransactionActionKey({ transactionHash: '0x1', transaction: { data: '0x' } });
const get = await getTransactionActionKey({ transactionHash: '0x1', transaction: { data: '0x', to: '0x0' } });
expect(get).toEqual(SEND_ETHER_ACTION_KEY);
});
it('getTransactionActionKey send token', async () => {
const get = await getTransactionActionKey({ transactionHash: '0x2', transaction: { data: transferData } });
const get = await getTransactionActionKey({
transactionHash: '0x2',
transaction: { data: transferData, to: '0x0' }
});
expect(get).toEqual(SEND_TOKEN_ACTION_KEY);
});

it('getTransactionActionKey send collectible', async () => {
const get = await getTransactionActionKey({ transactionHash: '0x6', transaction: { data: transferFromData } });
const get = await getTransactionActionKey({
transactionHash: '0x6',
transaction: { data: transferFromData, to: '0x0' }
});
expect(get).toEqual(TRANSFER_FROM_ACTION_KEY);
});

Expand Down
93 changes: 37 additions & 56 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading