-
Notifications
You must be signed in to change notification settings - Fork 15
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good! The only thing I think we need to change is to not remove the SLOAD/SSTORE gas costs.
fhevm/interface.go
Outdated
@@ -32,6 +32,7 @@ type EVMEnvironment interface { | |||
CreateContract2(caller common.Address, code []byte, codeHash common.Hash, gas uint64, value *big.Int, address common.Address) ([]byte, common.Address, uint64, error) | |||
|
|||
GetFhevmData() *FhevmData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe we can rename GetFhevmData()
to FhevmData()
too. Not part of this PR, just noticed.
fhevm/params.go
Outdated
// TODO: The values here are chosen somewhat arbitrarily (at least the 8 bit ones). Also, we don't | ||
// take into account whether a ciphertext existed (either "current" or "original") for the given handle. | ||
// Finally, costs are likely to change in the future. | ||
FheUint8ProtectedStorageSstoreGas uint64 = EvmNetSstoreInitGas + 2000 | ||
FheUint16ProtectedStorageSstoreGas uint64 = FheUint8ProtectedStorageSstoreGas * 2 | ||
FheUint32ProtectedStorageSstoreGas uint64 = FheUint16ProtectedStorageSstoreGas * 2 | ||
|
||
// TODO: We don't take whether the slot is cold or warm into consideration. | ||
FheUint8ProtectedStorageSloadGas uint64 = ColdSloadCostEIP2929 + 200 | ||
FheUint16ProtectedStorageSloadGas uint64 = FheUint8ProtectedStorageSloadGas * 2 | ||
FheUint32ProtectedStorageSloadGas uint64 = FheUint16ProtectedStorageSloadGas * 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not remove these. Instead, if they are unused, I think it indicates another issue in that we don't charge for protected storage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restored
@dartdart26 restored deleted constants, renamed also |
Adds clear api to customize fhevm-go, user needs to provide *FhevmParams struct, defaults exist but can be customized by user
@dartdart26 @youben11