Skip to content
This repository has been archived by the owner on Dec 23, 2024. It is now read-only.

Custom parameters for fhevm-go #31

Merged
merged 3 commits into from
Nov 8, 2023
Merged

Custom parameters for fhevm-go #31

merged 3 commits into from
Nov 8, 2023

Conversation

david-zk
Copy link
Contributor

@david-zk david-zk commented Nov 4, 2023

Adds clear api to customize fhevm-go, user needs to provide *FhevmParams struct, defaults exist but can be customized by user

@dartdart26 @youben11

@david-zk david-zk requested a review from dartdart26 November 4, 2023 04:17
Copy link
Contributor

@dartdart26 dartdart26 left a 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.

@@ -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
Copy link
Contributor

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
Comment on lines 94 to 104
// 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
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored

@david-zk
Copy link
Contributor Author

david-zk commented Nov 8, 2023

@dartdart26 restored deleted constants, renamed also GetFhevmData to FhevmData and GetFhevmParams to FhevmParams

@david-zk david-zk merged commit 41f7a61 into main Nov 8, 2023
1 check passed
@david-zk david-zk deleted the fix/custom-params branch November 8, 2023 08:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants