fix: eth_sendRawTransaction now handles malformed RLP data correctly

packages/relay/src/lib/precheck.ts

@@ -36,7 +36,11 @@ export class Precheck {
    * @returns {Transaction} The parsed transaction.
    */
   public static parseTxIfNeeded(transaction: string | Transaction): Transaction {
-    return typeof transaction === 'string' ? Transaction.from(transaction) : transaction;
+    try {
+      return typeof transaction === 'string' ? Transaction.from(transaction) : transaction;
+    } catch (e: any) {
+      throw predefined.INVALID_ARGUMENTS(e.message.toString());
+    }
   }
 
   /**

packages/relay/tests/lib/eth/eth_sendRawTransaction.spec.ts

@@ -125,8 +125,8 @@ describe('@ethSendRawTransaction eth_sendRawTransaction spec', async function ()
       sdkClientStub = sinon.createStubInstance(SDKClient);
       sinon.stub(hapiServiceInstance, 'getSDKClient').returns(sdkClientStub);
       restMock.onGet(accountEndpoint).reply(200, JSON.stringify(ACCOUNT_RES));
-   JSON.stringify(   restMock.onGet(receiverAccountEndpoint).reply(200, JSON.stringify(RECEIVER_ACCOUNT_RES)));
-   JSON.stringify(   restMock.onGet(networkExchangeRateEndpoint).reply(200, JSON.stringify(mockedExchangeRate)));
+      JSON.stringify(restMock.onGet(receiverAccountEndpoint).reply(200, JSON.stringify(RECEIVER_ACCOUNT_RES)));
+      JSON.stringify(restMock.onGet(networkExchangeRateEndpoint).reply(200, JSON.stringify(mockedExchangeRate)));
     });
 
     afterEach(() => {
@@ -195,6 +195,18 @@ describe('@ethSendRawTransaction eth_sendRawTransaction spec', async function ()
       );
     });
 
+    it('should return a predefined INVALID_ARGUMENTS when transaction has invalid format', async function () {
+      const invalidTx =
+        '0xf8748201280585800e8dfc0085800e8dfc00832dc6c094aca85ef7e1fce27079bbf99b60fcf6fd19b99b248502540be40080c001a0210446cfb671c3174392410d52fa3cd58723d8417e40cc67c6225b8f7e3ff693a02674b392846c59f783ea96655d39560956dd987051972064a5d853cea0b6f6d711';
+      await RelayAssertions.assertRejection(
+        predefined.INVALID_ARGUMENTS('invalid hex string'),
+        ethImpl.sendRawTransaction,
+        false,
+        ethImpl,
+        [invalidTx, requestDetails],
+      );
+    });
+
     it('should return a computed hash if unable to retrieve EthereumHash from record due to contract revert', async function () {
       const signed = await signTransaction(transaction);
 

packages/relay/tests/lib/precheck.spec.ts

@@ -57,6 +57,8 @@ describe('Precheck', async function () {
   const txWithZeroValue =
     '0xf86380843b9aca00825208940000000000000000000000000000000000000000808025a04e557f2008ff383df9a21919860939f60f4c27b9c845b89021ae2a79be4f6790a002f86d6dcefd2ffec72bf4d427091e7375acb6707e49d99893173cbc03515fd6';
   const parsedTxWithZeroValue = ethers.Transaction.from(txWithZeroValue);
+  const invalidTx =
+    '6080604052348015600f57600080fd5b50603f80601d6000396000f3fe6080604052600080fdfea26469706673582212209c06253b6069b4e1f720945c020dc1c7b3d74b850eba35ac8b6fb407eff7ca7364736f6c63430008120033';
 
   const defaultGasPrice = 720_000_000_000;
   const defaultGasLimit = 1_000_000;
@@ -782,4 +784,16 @@ describe('Precheck', async function () {
       expect(async () => await precheck.receiverAccount(parsedTx, requestDetails)).not.to.throw;
     });
   });
+
+  describe('parseTxIfNeeded', async function () {
+    it('should successfully parse a valid transaction string', function () {
+      const parsedTx = Precheck.parseTxIfNeeded(parsedTxWithMatchingChainId);
+      expect(parsedTx).to.be.instanceOf(Transaction);
+      expect(parsedTx.chainId).to.equal(BigInt(298));
+    });
+
+    it('should throw INVALID_ARGUMENTS for invalid RLP', function () {
+      expect(() => Precheck.parseTxIfNeeded(invalidTx)).to.throw('Invalid arguments: invalid BytesLike value');
+    });
+  });
 });

packages/server/tests/acceptance/rpc_batch1.spec.ts

@@ -1541,6 +1541,17 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () {
           await Assertions.assertPredefinedRpcError(error, sendRawTransaction, true, relay, [signedTx, requestDetails]);
         });
 
+        it('should fail "eth_sendRawTransaction" when transaction has invalid format', async function () {
+          const invalidTx =
+            '0xf8748201280585800e8dfc0085800e8dfc00832dc6c094aca85ef7e1fce27079bbf99b60fcf6fd19b99b248502540be40080c001a0210446cfb671c3174392410d52fa3cd58723d8417e40cc67c6225b8f7e3ff693a02674b392846c59f783ea96655d39560956dd987051972064a5d853cea0b6f6d711';
+          const error = predefined.INVALID_ARGUMENTS('unexpected junk after rlp payload');
+
+          await Assertions.assertPredefinedRpcError(error, sendRawTransaction, true, relay, [
+            invalidTx,
+            requestDetails,
+          ]);
+        });
+
         it('should fail "eth_sendRawTransaction" for EIP155 transaction with not enough gas', async function () {
           const gasLimit = 100;
           const transaction = {

packages/ws-server/tests/acceptance/sendRawTransaction.spec.ts

@@ -39,10 +39,8 @@ describe('@web-socket-batch-2 eth_sendRawTransaction', async function () {
   const requestId = 'sendRawTransactionTest_ws-server';
   const requestDetails = new RequestDetails({ requestId: requestId, ipAddress: '0.0.0.0' });
 
-  let tx: any,
-    sendHbarToProxyContractDeployerTx: any,
-    accounts: AliasAccount[] = [],
-    ethersWsProvider: WebSocketProvider;
+  let tx: any, sendHbarToProxyContractDeployerTx: any, ethersWsProvider: WebSocketProvider;
+  const accounts: AliasAccount[] = [];
 
   before(async () => {
     const initialAccount: AliasAccount = global.accounts[0];
@@ -112,6 +110,16 @@ describe('@web-socket-batch-2 eth_sendRawTransaction', async function () {
       expect(fromAccountInfo.evm_address).to.eq(accounts[0].address.toLowerCase());
     });
 
+    it(`Should fail eth_sendRawTransaction on Standard Web Socket when transaction has invalid format`, async () => {
+      const invalidTx =
+        '0xf8748201280585800e8dfc0085800e8dfc00832dc6c094aca85ef7e1fce27079bbf99b60fcf6fd19b99b248502540be40080c001a0210446cfb671c3174392410d52fa3cd58723d8417e40cc67c6225b8f7e3ff693a02674b392846c59f783ea96655d39560956dd987051972064a5d853cea0b6f6d711';
+      const response = await WsTestHelper.sendRequestToStandardWebSocket(METHOD_NAME, [invalidTx], 1000);
+
+      const expectedError = predefined.INVALID_ARGUMENTS('unexpected junk after rlp payload');
+      expect(response.error.code).to.eq(expectedError.code);
+      expect(response.error.message).to.contain(expectedError.message);
+    });
+
     it(`Should execute eth_sendRawTransaction on Standard Web Socket for the deterministic deployment transaction`, async () => {
       // send gas money to the proxy deployer
       sendHbarToProxyContractDeployerTx.nonce = await global.relay.getAccountNonce(accounts[0].address);
@@ -178,6 +186,19 @@ describe('@web-socket-batch-2 eth_sendRawTransaction', async function () {
       expect(fromAccountInfo.evm_address).to.eq(accounts[1].address.toLowerCase());
     });
 
+    it(`Should fail eth_sendRawTransaction on Ethers Web Socket Provider when transaction has invalid format`, async () => {
+      const invalidTx =
+        '0xf8748201280585800e8dfc0085800e8dfc00832dc6c094aca85ef7e1fce27079bbf99b60fcf6fd19b99b248502540be40080c001a0210446cfb671c3174392410d52fa3cd58723d8417e40cc67c6225b8f7e3ff693a02674b392846c59f783ea96655d39560956dd987051972064a5d853cea0b6f6d711';
+      try {
+        await ethersWsProvider.send(METHOD_NAME, [invalidTx]);
+        expect.fail('Should have thrown an error');
+      } catch (error) {
+        const expectedError = predefined.INVALID_ARGUMENTS('invalid hex string');
+        expect(error.info.error.code).to.eq(expectedError.code);
+        expect(error.info.error.message).to.contain(expectedError.message);
+      }
+    });
+
     it(`Should execute eth_sendRawTransaction on Ethers Web Socket Provider for the deterministic deployment transaction`, async () => {
       // send gas money to the proxy deployer
       sendHbarToProxyContractDeployerTx.nonce = await global.relay.getAccountNonce(accounts[1].address);