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

JsonRpcProvider.hexlifyTransaction removes chainId which leads to EIP-155 error #2691

Closed
moltam89 opened this issue Feb 14, 2022 · 7 comments
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@moltam89
Copy link

In JsonRpcProvider.hexlifyTransaction chanId is not listed, therefore the transaction will be sent without it, which can lead to EIP-155 error.

I'm not sure if this is intentional or not, allowedTransactionKeys already contains the chainId, so this might be a bug.

This error can be reproduced using https://punkwallet.io/ on Polygon, though this might be an error on their side.

@moltam89 moltam89 added the investigate Under investigation and may be a bug. label Feb 14, 2022
@moltam89
Copy link
Author

Hey @ricmoo ,

Do you think this is a bug?

Thanks,
Tamas

@ricmoo ricmoo added the on-deck This Enhancement or Bug is currently being worked on. label Feb 19, 2022
@ricmoo
Copy link
Member

ricmoo commented Feb 19, 2022

I think it may be a bug, but need to look more into it.

If so, it likely hasn't come up before as chainId is generally not something JsonRpcSigners should be passing, since their client will know better (similar to why nonce isn't generally passed in).

I'll look into this for the upcoming minor bump. :)

@moltam89
Copy link
Author

Thanks @ricmoo

I'm not sure if it helps, but I extracted the logic from https://punkwallet.io/ to make it clearer what is happening.
This code fails with EIP155 error.
When I add the chainId back, after hexlifyTransaction removed it, the transaction is working fine.

import React from "react";
import { Web3Provider } from "@ethersproject/providers";

const ethers = require('ethers');
const ProviderEngine = require('web3-provider-engine')

const HookedWalletSubprovider = require('web3-provider-engine/subproviders/hooked-wallet-ethtx.js')
const RpcSubprovider = require('web3-provider-engine/subproviders/rpc.js')

const sigUtil = require('eth-sig-util')

export default function EIP155({}) {
  let engine = new ProviderEngine()
  let provider = new ethers.providers.Web3Provider(engine)

  let privateKey = "*************"    
  let wallet = new ethers.Wallet(privateKey, provider);
  
  let opts = {rpcUrl: 'https://polygon-rpc.com'};

	opts.getPrivateKey = (address,cb)=>{
	  cb(null,Buffer.from(wallet.privateKey.replace("0x",""),'hex'))
	};

	opts.getAccounts = (cb)=>{
	  cb(false,[wallet.address])
	};

  const hookedWalletSubprovider = new HookedWalletSubprovider(opts);

  hookedWalletSubprovider.signTypedMessage = function (msgParams, cb) {
    opts.getPrivateKey(msgParams.from, function(err, privateKey) {
      if (err) return cb(err)

      if (typeof msgParams.data === 'string') {
        msgParams.data = JSON.parse(msgParams.data);
      }

      const serialized = sigUtil.signTypedData_v4(privateKey, msgParams)
      cb(null, serialized)
    })
  }
  engine.addProvider(hookedWalletSubprovider)

  engine.addProvider(new RpcSubprovider(opts))

  engine.start()

  const web3Provider = new Web3Provider(engine);

  const txData = {
    to: "0xc54C244200d657650087455869f1aD168537d3B3",
    value: "0x01",
    chainId:137,
  }

   const signer = web3Provider.getSigner();

  signer.sendTransaction(txData)
  .then((tx) => {
    console.log("tx", tx);
  })
  .catch(error => {
  	console.log("error", error);
  });

  return (
     <div>
      EIP155
     </div>
  );
}

@ricmoo
Copy link
Member

ricmoo commented Feb 22, 2022

I've added this in my local branch for v5.6 coming out (hopefully) later this week.

Thanks! :)

@ricmoo ricmoo added bug Verified to be an issue. and removed investigate Under investigation and may be a bug. labels Feb 22, 2022
@ricmoo
Copy link
Member

ricmoo commented Mar 10, 2022

This should be fixed in v5.6.

Try it out and let me know if you still have any issues. :)

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Mar 10, 2022
@moltam89
Copy link
Author

Yep, the fix works.
Thank you!

@ricmoo
Copy link
Member

ricmoo commented Mar 10, 2022

Awesome! Glad to hear it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

2 participants