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

[PoC] Native short cuts experiment #595

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

birchmd
Copy link
Member

@birchmd birchmd commented Aug 31, 2022

Background

We recently updated our SputnikVM version to include a recent change that makes precompiles able to make direct EVM calls using a handle object. This object effectively hooks directly into the EVM runtime, which got me thinking that it would be easy to give the handle access to EVM state as well. Then a precompile would be able to do anything a normal contract is able to. Therefore, the precompiles mechanism could be used to make "native short-cuts" we have discussed previously (the idea the we could have performance improvements by skipping over the EVM interpreter through executing equivalent native code directly -- previous results have shown native code can vastly outperform EVM interpretation).

Description

In a separate commit I added state access to the PrecompileHandle trait in SputnikVM. We could make this into a proper PR if we think this approach is worth pursuing.

In this PR I use this new ability to make a native implementation of the ERC-20 standard which is binary-compatible with the Solidity version (i.e. reads and writes the EVM state in exactly the same way). Additionally, I add some functionality to the engine to dynamically assign addresses to precompiles. This allows deployed ERC-20 contracts to be replaced with a "native short-cut". Then I test out this new feature by short-cutting all the ERC-20 tokens used in our Uniswap tests.

Performance / NEAR gas cost considerations

The entire purpose of this change is to give us another tool we can use for performance optimizations. In the case of test_uniswap_input_multihop (our most expensive uniswap test), we see about a 15% reduction in the amount of NEAR gas used. Honestly I was hoping for a bigger impact given how much effort it is to translate even a simple Solidity contract into a native counterpart. I suspect much of the gas cost still comes from state access and the overhead involved with making calls in the EVM (each call creates a new stack frame inside SputnikVM, which is needed to respect revert semantics properly, and this is a fairly computationally expensive operation).

Some tests actually now use a little more NEAR gas with this change due to the additional state read which is required to look for the new dynamic precompiles ("native contracts").

How should this be reviewed

This PR is a PoC, not intended for production in its current form. As discussed below, there are a number of considerations when it comes to deciding if we want to adopt a feature like this, and if we do adopt it, what contracts we want to give native short-cuts to.

In particular, the reviewer should not focus on the details of the native ERC-20 implementation (for example, I did not implement the events the Solidity contract emits, and I think there are probably some edge case incompatibilities which would need to be ironed out if we wanted this in production). Instead, please focus on the design of the feature as a whole, and the high level ideas of translating a contract from Solidity into this Rust dialect.

Additional information

The purpose of this PoC is research. The main deliverable from this PR is us deciding if we want to spend more time pursing this kind of feature. What follows are some pros and cons I can think of, but please give your own opinions in your reviews!

Pros

  • Performance improvement for short-cut contracts. 15% on the ERC-20 example is less than I hoped for, but certainly not nothing! And presumably more computationally intensive contracts would have more opportunities for gains.
  • Potentially adds Rust to the list of languages you can write Aurora contracts in. I focused on translating a Solidity contract for this PoC because I think that would be the main use-case for this feature in the short-term, however we do not need to tether ourselves to the Solidity ABI. One could write a whole contract from scratch using this approach, and some partners may be interested in doing this. By writing it from scratch there are more opportunities for performances improvements. For example, with the ERC-20 balance/allowance mappings, the slot numbers are 1 byte and the address keys are 20 bytes, so using the keccak hash to make 32-byte keys is not necessary, thus saving some computation if we are willing to break the Solidity ABI.

Cons

  • Translating Solidity contracts creates additional auditing burdens. Effectively this requires re-implementing any contract we want to short-cut in its entirety in Rust. And if we want to upgrade an existing contract, then the Rust implementation needs to be binary-compatible with the Solidity version. Doing this for mission-critical contract (eg the bridged ERC-20 tokens) is dangerous and would require lots of testing and audits to ensure we get it right. Its possible this drawback could be mitigated through an automated transpiler from Solidity to Rust (something along the lines of what @artob has been working on, but at a higher level than the bytecode), however such a tool would be a non-trivial development effort and would itself require auditing.
  • Gas cost of transactions not touching one of the short-cut contracts would be more expensive (due to the state read of the new dynamic precompiles list). This drawback could be mitigated if we short-cut all the "hot" contracts that get used the most often. The bridged ERC-20 tokens are one place to start, but we would probably need to get partners (like Trisolaris, Aurigami and Bastion) on board with this approach if we wanted to reach all the most popular contracts.

@birchmd birchmd added C-research Category: Research, may not end up as a feature. S-do-not-merge Status: Do not merge A-benchmark Area: performance benchmarks labels Aug 31, 2022
@birchmd birchmd requested review from artob and mfornet August 31, 2022 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-benchmark Area: performance benchmarks C-research Category: Research, may not end up as a feature. S-do-not-merge Status: Do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant