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

fix: added support for legacy pre EIP 155 transactions with no chainID (chainId=0x0) #2228

Merged
merged 4 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion packages/relay/src/lib/precheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export class Precheck {
chainId(tx: Transaction, requestId?: string) {
const requestIdPrefix = formatRequestIdMessage(requestId);
const txChainId = prepend0x(Number(tx.chainId).toString(16));
const passes = txChainId === this.chain;
const passes = txChainId === '0x0' || txChainId === this.chain;
Nana-EC marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

should we tighten this up a bit by checking to see if the v value is 27 or 28 (for legacy transactions) when the chain_id is zero?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this makes sense! Just pushed in an update for it

if (!passes) {
this.logger.trace(
`${requestIdPrefix} Failed chainId precheck for sendRawTransaction(transaction=%s, chainId=%s)`,
Expand Down
15 changes: 13 additions & 2 deletions packages/relay/tests/lib/precheck.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,11 @@ describe('Precheck', async function () {
'0x02f87482012a0485a7a358200085a7a3582000832dc6c09400000000000000000000000000000000000003f78502540be40080c001a006f4cd8e6f84b76a05a5c1542a08682c928108ef7163d9c1bf1f3b636b1cd1fba032097cbf2dda17a2dcc40f62c97964d9d930cdce2e8a9df9a8ba023cda28e4ad';
const parsedTxWithMatchingChainId = ethers.Transaction.from(txWithMatchingChainId);
const parsedTxGasPrice = 1440000000000;
const txWithNonMatchingChainId =
const txWithChainId0x0 =
'0xf86a0385a7a3582000832dc6c09400000000000000000000000000000000000003f78502540be400801ca06750e92db52fa708e27f94f27e0cfb7f5800f9b657180bb2e94c1520cfb1fb6da01bec6045068b6db38b55017bb8b50166699384bc1791fd8331febab0cf629a2a';
const parsedtxWithChainId0x0 = ethers.Transaction.from(txWithChainId0x0);
const txWithNonMatchingChainId =
'0xf86c8085a54f4c3c00832dc6c094000000000000000000000000000000000000042f8502540be40080820306a0fe71ab0077a58d112eecc7f95b9a7563ffdc14a45440cc1b2c698dbb1a687abea063ba3725ae54118f45999f5b53b38ba67b61f2365965784a81b9b47f37b78c10';
const parsedTxWithNonMatchingChainId = ethers.Transaction.from(txWithNonMatchingChainId);
const txWithValueMoreThanOneTinyBar =
'0xf8628080809449858d4445908c12fcf70251d3f4682e8c9c381085174876e800801ba015ec73d3e329c7f5c0228be39bf30758f974d69468847dd507082c89ec453fe2a04124cc1dd6ac07417e7cdbe04cb99d698bddc6ce4d04054dd8978dec3493f3d2';
Expand Down Expand Up @@ -133,14 +136,22 @@ describe('Precheck', async function () {
}
});

it('should pass when chainId=0x0', async function () {
try {
precheck.chainId(parsedtxWithChainId0x0);
} catch (e: any) {
expect(e).to.not.exist;
}
});

it('should not pass for non-matching chainId', async function () {
try {
precheck.chainId(parsedTxWithNonMatchingChainId);
expectedError();
} catch (e: any) {
expect(e).to.exist;
expect(e.code).to.eq(-32000);
expect(e.message).to.eq('ChainId (0x0) not supported. The correct chainId is 0x12a');
expect(e.message).to.eq('ChainId (0x171) not supported. The correct chainId is 0x12a');
}
});
});
Expand Down
13 changes: 10 additions & 3 deletions packages/server/tests/acceptance/rpc_batch1.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -828,17 +828,24 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () {
await Assertions.assertPredefinedRpcError(error, sendRawTransaction, true, relay, [signedTx, requestId]);
});

it('should fail "eth_sendRawTransaction" for Legacy transactions (with no chainId)', async function () {
it('should execute "eth_sendRawTransaction" for legacy transactions (with no chainId i.e. chainId=0x0)', async function () {
const receiverInitialBalance = await relay.getBalance(mirrorContract.evm_address, 'latest', requestId);
const transaction = {
...defaultLegacyTransactionData,
to: mirrorContract.evm_address,
nonce: await relay.getAccountNonce(accounts[2].address, requestId),
gasPrice: await relay.gasPrice(requestId),
};
const signedTx = await accounts[2].wallet.signTransaction(transaction);
const error = predefined.UNSUPPORTED_CHAIN_ID('0x0', CHAIN_ID);
const transactionHash = await relay.sendRawTransaction(signedTx, requestId);
// Since the transactionId is not available in this context
// Wait for the transaction to be processed and imported in the mirror node with axios-retry
await new Promise((r) => setTimeout(r, 5000));
await mirrorNode.get(`/contracts/results/${transactionHash}`, requestId);

await Assertions.assertPredefinedRpcError(error, sendRawTransaction, true, relay, [signedTx, requestId]);
const receiverEndBalance = await relay.getBalance(mirrorContract.evm_address, 'latest', requestId);
const balanceChange = receiverEndBalance - receiverInitialBalance;
expect(balanceChange.toString()).to.eq(Number(ONE_TINYBAR).toString());
});

it('should fail "eth_sendRawTransaction" for Legacy transactions (with gas price too low)', async function () {
Expand Down
Loading