search

aurora-is-near / aurora-engine

⚙️ Aurora Engine implements an Ethereum Virtual Machine (EVM) on the NEAR Protocol.
https://doc.aurora.dev/develop/compat/evm
325 stars 78 forks source link

Fix: remove unused state variable `bridge_prover_id` #749

Closed birchmd closed 1 year ago

birchmd commented 1 year ago

This PR is a modified version of #726

Normally I would not make a whole separate PR, but I ended up starting this work from scratch off of develop because I could not understand why the gas costs in repro were increased so much in the old PR. I have since figured out that it was some change in Cargo.lock that was causing the problem, but by that point I already had all this code finished in a separate branch so I thought it would be easier to make a separate PR at that point.

Key differences from #726 :

  1. The repro tests pass without any changes to the gas values (i.e. this implementation does not create a significant gas increase). This is mostly due to leaving Cargo.lock untouched, but there are a few other improvements as well (see below).
  2. Automatically migrate the state if the V1 state is found. This change means that after the first transaction, the fallback deserialization logic will not be executed and hence reading the state will be as efficient as it always was (as exemplified by the repro gas costs remaining unchanged).
  3. Use Cow in BorshableEngineState so that From<&EngineState> for BorshableEngineState does not need to clone the owner account id.
  4. Ensure removing the bridge_prover_id field from NewCallArgs is done in a backwards compatible way. This is not a performance improvement, but it is still a critical change. We must ensure that changes to the format of top-level contract functions are always backwards compatible otherwise the Borealis Engine and Borealis Refiner will not be able to parse old transactions (it will fail to deserialize the arguments).
aleksuss commented 1 year ago

I'm really curious about the changes in the Cargo.lock, which increases the gas cost. Were you able to find out what they are?

birchmd commented 1 year ago

I'm really curious about the changes in the Cargo.lock, which increases the gas cost. Were you able to find out what they are?

I didn't dig into it further because there were quite a few changes. I just reverted the whole file back to the version on develop and the gas issues went away. But I agree that it would be nice to figure out exactly which dependency was the issue.