Ethereum transactions aren't always being treated as EIP 1559 compliant when they are

[scotthconner]

Checklist

• This is not a security-related bug/issue. If it is, please follow please follow the security policy.
• This is not a question or a support request. If you have any lotus related questions, please ask in the lotus forum.
• This is not a new feature request. If it is, please file a feature request instead.
• This is not an enhancement request. If it is, please file a improvement suggestion instead.
• I have searched on the issue tracker and the lotus forum, and there is no existing related issue or discussion.
• I am running the Latest release, or the most recent RC(release canadiate) for the upcoming release or the dev branch(master), or have an issue updating to any of these.
• I did not make any code changes to lotus.

Lotus component

• lotus daemon - chain sync
• lotus miner - mining and block production
• lotus miner/worker - sealing
• lotus miner - proving(WindowPoSt)
• lotus miner/market - storage deal
• lotus miner/market - retrieval deal
• lotus miner/market - data transfer
• lotus client
• lotus JSON-RPC API
• lotus message management (mpool)
• Other

Lotus Version

FEVM wallaby

Describe the Bug

Ethereum transactions are required to manually set the maxPriorityGas, which is divergent from the backwards compatible nature of ethereum nodes and ethers/web3js support.

Without setting the fee explicitly, the node fails to add the transaction to the mempool as a "legacy transaction."

I've narrowed down the gas issue to ethtypes.ParseEthTxArgs checking the first byte slot for the value of 2 for EIP 1559. Here are the transactionRequests from one that failed (first), and another that succeeded (second):

Success (this is using explicit fee overrides in deploy script ala 00_deploy.js:

{
  data: '0x2e9d4d850000000000000000000000005e2918a472611f4e9eb50ff4064b63106770f6c3',
  to: '0x718CB9B43E9DdB697F82B7F69800EA1fCaFF239c',
  from: '0x4E5d95F1D3d1b1FB4a169554A6bff1fD164ACa2c',
  type: 2,
  maxFeePerGas: BigNumber { value: "1000000000" },
  maxPriorityFeePerGas: BigNumber { value: "1000000000" },
  nonce: 41,
  gasLimit: BigNumber { value: "7644325" },
  chainId: 31415926
}

Failure: In this one, I just let ethers do its magic. I've confirmed that its calling estimateGas, and maxPriorityFee and are getting correct values out of it.

{
  data: '0x2e9d4d850000000000000000000000005e2918a472611f4e9eb50ff4064b63106770f6c3',
  to: '0x718CB9B43E9DdB697F82B7F69800EA1fCaFF239c',
  from: '0x4E5d95F1D3d1b1FB4a169554A6bff1fD164ACa2c',
  type: 2,
  maxFeePerGas: BigNumber { value: "1500000200" },
  maxPriorityFeePerGas: BigNumber { value: "1500000000" },
  nonce: 41,
  gasLimit: BigNumber { value: "7644325" },
  chainId: 31415926
}

In the failure case, I've made sure that the node is getting hit and returning valid values for gas estimation and priority fee. In both cases, the transaction is properly assigned as type 2 (EIP 1559). The transaction data is call data is identical. By the surface, the only thing that differs is the value.

In digging into the bytes being decoded on the node, you can see the first few bits of data in the failure transaction are not in fact "type 2".

[248 139 31 131 3 5 239 131 116 164 165 148 113 140 185 180 62 157 219 105 127 130 183 246 152 0 234 31 202 255 35 156 128 164 46 157 77 133 0 0 0 0 0 0 0 0 0 0 0 0 94 41 24 164 114 97 31 78 158 181 15 244 6 75 99 16 103 112 246 195 132 3 190 189 15 160 129 255 40 246 168 145 36 194 134 80 153 79 82 208 133 77 249 58 157 53 27 89 112 175 149 22 137 53 237 29 17 214 160 116 35 106 122 103 130 142 248 214 66 90 196 38 164 16 57 219 246 130 102 33 35 22 79 108 214 152 120 209 41 107 57]

However, for some reason, when setting the fee data explicitly, the first byte is 2.

[2 248 147 132 1 223 94 118 31 132 59 154 202 0 132 59 154 202 0 131 116 164 165 148 113 140 185 180 62 157 219 105 127 130 183 246 152 0 234 31 202 255 35 156 128 164 46 157 77 133 0 0 0 0 0 0 0 0 0 0 0 0 94 41 24 164 114 97 31 78 158 181 15 244 6 75 99 16 103 112 246 195 192 1 160 215 221 38 234 239 195 159 158 214 146 71 238 29 185 17 180 229 217 129 1 71 113 132 163 147 255 163 140 44 71 6 39 160 38 28 181 148 86 161 26 186 117 42 230 179 119 239 38 47 159 242 195 159 8 154 72 255 216 55 93 83 253 73 214 23]

You can find the deploy script, and the relevant FEVM-specific things that need to be done (otherwise tested against goerli and mumbai with no issues) here.

Logging Information

I've provided sufficient information above.

Repo Steps

One thing to note is I'm running some cherry-picks, but this behavior was observed on wallaby as well.

[scotthconner]

@raulk @jennijuju @geoff-vball

[scotthconner]

Seems related to the discussion in #fil-fvm-dev: https://filecoinproject.slack.com/archives/C029MT4PQB1/p1668861006243529

[scotthconner]

Here is the full output from my deployer:

smartrust git:(main) ✗ npx hardhat respect --network devnet

==== GENIE, RESPECT! ====

{}

=== SIGNER INFO ===

Signer Network Chain ID: 31415926
Signer Wallet Address: 0x4E5d95F1D3d1b1FB4a169554A6bff1fD164ACa2c

=== CONTRACT INFO ===

KeyVault: 0x718CB9B43E9DdB697F82B7F69800EA1fCaFF239c
Locksmith: 0x5e2918a472611f4e9eB50fF4064b63106770F6C3

=== Calling setRespectedLocksmith... ===

The current respect address is: 0x5e2918a472611f4e9eB50fF4064b63106770F6C3
The current locksmith is already respected!
{
 data: '0x2e9d4d850000000000000000000000005e2918a472611f4e9eb50ff4064b63106770f6c3',
 to: '0x718CB9B43E9DdB697F82B7F69800EA1fCaFF239c',
 from: '0x4E5d95F1D3d1b1FB4a169554A6bff1fD164ACa2c',
 type: 2,
 maxFeePerGas: BigNumber { value: "1500000200" },
 maxPriorityFeePerGas: BigNumber { value: "1500000000" },
 nonce: 45,
 gasLimit: BigNumber { value: "7644325" },
 chainId: 31415926
}
ERROR!
legacy transaction is not supported

[scotthconner]

I've also just verified via WireShark and Lotus debugging that the hex getting sent from the client (ethers) is in fact what is getting analyzed on the server side.

[scotthconner]

I'm wondering if there is something about ethers that is setting the type to something that is > 0x7f according to the code to consider it a legacy transaction. What is concerning is the transaction type is set as 2 on the way out.

Can you help me understand the implementation that was provided here? #9334 @raulk

[scotthconner]

Here is raw params to eth_sendRawTransaction for a transaction that didn't work (not manually setting maxFeePerGas, but letting ethers.js do it and seeing it prepare a type 2 transaction:

0xf88b2e83030cdb8374a4a594718cb9b43e9ddb697f82b7f69800ea1fcaff239c80a42e9d4d850000000000000000000000005e2918a472611f4e9eb50ff4064b63106770f6c38403bebd0fa088f7f094baf063874221dcf96bb6f49101f1d1c7265949f2d794d39799be3732a072a9a44b4a9f4b13e160f64013d7bc12ebe24c1118022dc318c7df01019f554f

There is the raw params for one that did work, by manually overriding the ethers provider and providing essentially the exact same information, also inspected to be type 2 on the way out.

0x02f8938401df5e761f843b9aca00843b9aca008374a4a594718cb9b43e9ddb697f82b7f69800ea1fcaff239c80a42e9d4d850000000000000000000000005e2918a472611f4e9eb50ff4064b63106770f6c3c001a0d7dd26eaefc39f9ed69247ee1db911b4e5d98101477184a393ffa38c2c470627a0261cb59456a11aba752ae6b377ef262f9ff2c39f089a48ffd8375d53fd49d617

This second one I can see does indeed have '2' is the leading byte. I suspect there is a dual part interaction to what is happening here:

1. Something about not setting it manually is still signing ethers providers without EIP 1559 transactions, or so it seems. According to the implementation history (linked above in the OP), they considered not having to set this as the right design decision. I've also noticed other folks in Filecoin ecosystem have run into this, so its not something just I seem to be doing wrong.
2. Most EVM implementations seem to have taken the legacy transaction support path, preventing the need to manually set gas unless its something you wanted to do.

[ychiaoli18]

Thanks for providing the additional information. Ethers.js did sign your first tx in the legacy format, even though you explicitly said that it should be a type 2 transaction. I agree with you that this seems to be an important issue, and we should spend some time digging into ethers.js's logic to understand how it determines the type of the transaction it signs, and we may want to support legacy format as well.

[scotthconner]

Yeah I'm trying to trace it back now. I'll let you know what I find.

[scotthconner]

I've actually narrowed it down to using this line or not:

owner = new ethers.Wallet(process.env.MY_PRIVATE_KEY, ethers.provider);

Manually setting the gas price isn't needed if I transform the default SignerWithAddress with a Wallet instead (both utilizing the provider interface, it seems).

My intuition is something is going awry here: https://github.com/ethers-io/ethers.js/blob/master/packages/abstract-signer/src.ts/index.ts#L261

I need to understand why SignerWithAddress and Wallet behave differently. It likely also explains why the example sets a field as well.

[scotthconner]

One thing I'm considering is that by default, many hardhat developers simply override the first signer in the environment. This has many folks by default using the hardhat provider. There are some discussions about how hardhat manages EIP 1559 transactions and its a recent and open discussion: NomicFoundation/hardhat#3418

I haven't been able to wrestle it to the ground, but setting the signer as a wallet moves the signer/provider combo over to a pure ethers.js implementation versus using a hardhat extension. This alone seems to work and creates proper EIP 1559 transactions.

I'm going to verify that this is the case by trying to interact with it with a frontend over the next day or so, and if its a hardhat thing, I'll provide a way to resolve the issue without overloading gas on the transaction.

[scotthconner]

This is a good non-gas trick work around to avoid the problems with hardhat json rpc signers.

scotthconner/smartrust@f3a22fe#diff-6cecb7811933ed7b95527f4e95faf845d6dec19b51002944f9d7b8e7343b6515

Generally, instead of using hardhat's built in signers, we should be using the wallet signer with private keys. The general gist is here:

///////////////////////////////////////////
// patchOwner
//
// This is needed because hardhat doesn't always use EIP 1559
// when using the default JsonRPCSigner, and doesn't necessarily
// support the right over-rides in hardhat.config.js.
//
// https://github.com/NomicFoundation/hardhat/issues/3418
///////////////////////////////////////////
const patchOwner = async function() {
  const [signer] = await ethers.getSigners();

  // for networks that absolutely require type 2 transactions, avoid
  // the hardhat signer by creating a wallet.
  if ((await signer.getChainId()).toString().match(/31415/)) {
    return new ethers.Wallet(process.env.MY_PRIVATE_KEY, ethers.provider);
  }

  // for networks that are backwards compatible, allow
  // hardhat to do unenveloped transactions at the potential
  // of overpaying for gas
  return signer; 
}

While hardhat.config.js does let you import RPC urls and accounts, it's left me wondering if using the default signer in combination with ethers Contract abstractions (instead of building RPC calls directly) is putting developers into non EIP-1559 transactions and potentially paying more in gas.

I tried patching everything, but on the hardhat, goerli, and mumbai networks I would get timeouts by using a Wallet.

[jennijuju]

Closed by #9965