feat: stateful precompiles #42
Conversation
| // evm.precompileManager.Register(common.HexToAddress("0x1001"), compress.NewCompress()) | ||
| // evm.precompileManager.Register(common.HexToAddress("0x1002"), jsonutil.NewJsonUtil()) |
| // e.g. register native minter to 0x0000000000000000000000000000000000001000 | ||
| evm.precompileManager.Register( | ||
| common.HexToAddress("0x1000"), | ||
| nativeminter.NewNativeMinter(), | ||
| ) |
There was a problem hiding this comment.
do we want to have this functionality here? probably would be better to move to an example branch
There was a problem hiding this comment.
No, it def needs removed, at least registering one. Could probably just be moved to a precompile doc that explains a bit and shows examples
| // PrecompileManager registers and runs stateful precompiles | ||
| type PrecompileManager interface { | ||
| IsPrecompile(addr common.Address) bool | ||
| Run(addr common.Address, input []byte, caller common.Address, value *big.Int, suppliedGas uint64, readonly bool) (ret []byte, remainingGas uint64, err error) | ||
| Register(addr common.Address, p precompile.StatefulPrecompiledContract) error | ||
| } |
There was a problem hiding this comment.
why do we need the interface? are there multiple implementations of this?
There was a problem hiding this comment.
There are not. I don't recall why I made the interface. Will remove.
| return fmt.Errorf("precompiled contract already exists at address %v", addr.Hex()) | ||
| } | ||
|
|
||
| // niaeve implementation; parsed abi method names must match precompile method names 1:1 |
There was a problem hiding this comment.
| // niaeve implementation; parsed abi method names must match precompile method names 1:1 | |
| // naive implementation; parsed abi method names must match precompile method names 1:1 |
| StatefulPrecompiledContract: precompile.NewStatefulPrecompiledContract( | ||
| bindings.NativeMinterABI, | ||
| ), |
There was a problem hiding this comment.
where is this used? i don't see this used anywhere. i'm also somewhat confused on why there are both native and solidity implementations when it looks like the native is actually implementing the storage changes, not doing it via calling the contract
There was a problem hiding this comment.
is this just used for returning the ABI? if so I would change this to get rid of the solidity/generated bindings and just manually create a function in this file that returns the contract function signature -> native method
There was a problem hiding this comment.
The solidity interface serves to both be used for ABI gen (which is being used for the function mapping) and also as an integration point for other contracts to call the precompiles, e.g
nativeMinter = INativeMinter(_precompileAddr);
nativeMinter.mint(_recipient, _amount);There was a problem hiding this comment.
oh yeah the generated ABI is also being used to unpack the input to pass to the native method and pack the return into the output. The generated bindings aren't used at all tho, just the generated ABI string. Could maybe clean that up some by having the ABI as a const in the precompile contract native code.
| )) | ||
|
|
||
| // check if precompile returned an error | ||
| if len(results) > 0 { |
There was a problem hiding this comment.
can we check that the result parameters (length, type) are what are expected based on the method signature?
| output, err = method.abiMethod.Outputs.Pack(interfaceArgs...) | ||
| if err != nil { | ||
| return nil, 0, err | ||
| } |
There was a problem hiding this comment.
related to above - if the number/type of args aren't correct, this will probably error?
| } | ||
| } | ||
|
|
||
| suppliedGas -= gasCost |
There was a problem hiding this comment.
does the caller check if this went negative? probably but just want to confirm
There was a problem hiding this comment.
There's a check above, Line 77,
gasCost := contract.RequiredGas(input)
if gasCost > suppliedGas {
return nil, 0, ErrOutOfGas
}
| // The method name of the first one will be resolved as Foo while the second one | ||
| // will be resolved as Foo0. | ||
| // | ||
| // Alternatively could require each precompile to define the func mapping instead of doing this magic |
There was a problem hiding this comment.
i think this is a lot better - the solidity/abigen stuff is unused apart from the abi, so we should remove it and just have the native code define the mapping.
| return spc.abi | ||
| } | ||
|
|
||
| func (spc *statefulPrecompiledContract) RequiredGas(input []byte) uint64 { |
There was a problem hiding this comment.
the contract has multiple methods, shouldn't gas be defined for each method or specifically each state access?
There was a problem hiding this comment.
you technically can with this if you decode input, but that's some heavy lifting every time you want to have different gas for each method. I have yet to come up with a good way to allow each method to define it's own gas that doesn't use some reflect black magic. Open to ideas
Summary
Add ability for stateful precompiles into the evm.
Stateful precompiles have access to:
Includes example implementation of a native minter precompile