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.

Sign up for GitHub

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

Fixes gas estimation for ccrs #209

Merged
merged 4 commits into from
Feb 27, 2024
Merged

Fixes gas estimation for ccrs #209

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b1fb0fc
This PR fixes gas estimation for ccrs
ferranbt Feb 26, 2024
a1188d0
Change val
ferranbt Feb 26, 2024
fbd21bf
Add comments
ferranbt Feb 26, 2024
1298d22
Add t.Parallel
ferranbt Feb 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions internal/ethapi/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import (
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/rlp"
"github.com/ethereum/go-ethereum/rpc"
suave "github.com/ethereum/go-ethereum/suave/core"
"github.com/tyler-smith/go-bip39"
)

Expand Down Expand Up @@ -1028,6 +1027,7 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash
args.KettleAddress = &acc
}

args.setDefaults(ctx, b)
lthibault marked this conversation as resolved.
Show resolved Hide resolved
tx := args.ToTransaction()

state, header, err := b.StateAndHeaderByNumber(ctx, rpc.LatestBlockNumber)
Expand All @@ -1038,7 +1038,7 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash
msg := &core.Message{
Nonce: tx.Nonce(),
GasLimit: tx.Gas(),
GasPrice: new(big.Int),
GasPrice: tx.GasPrice(),
GasFeeCap: new(big.Int),
GasTipCap: new(big.Int),
To: tx.To(),
Expand All @@ -1048,13 +1048,10 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash
SkipAccountChecks: true,
}

_, result, finalize, err := runMEVM(ctx, b, state, header, tx, msg, true)
_, result, _, err := runMEVM(ctx, b, state, header, tx, msg, true)
lthibault marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
if err := finalize(); err != suave.ErrUnsignedFinalize {
return nil, err
}
return result, err
}

Expand Down Expand Up @@ -1927,10 +1924,13 @@ func (s *TransactionAPI) SendRawTransaction(ctx context.Context, input hexutil.B
return common.Hash{}, err
}

ntx, _, finalize, err := runMEVM(ctx, s.b, state, header, tx, msg, false)
ntx, result, finalize, err := runMEVM(ctx, s.b, state, header, tx, msg, false)
if err != nil {
return tx.Hash(), err
}
if result.Failed() {
return tx.Hash(), fmt.Errorf("transaction failed: %w", result.Err)
}
if err = finalize(); err != nil {
log.Error("could not finalize confidential store", "err", err)
return tx.Hash(), err
Expand Down Expand Up @@ -2017,7 +2017,7 @@ func runMEVM(ctx context.Context, b Backend, state *state.StateDB, header *types
}

if result.Failed() {
return nil, nil, nil, fmt.Errorf("%w: %s", result.Err, hexutil.Encode(result.Revert()))
return nil, result, storeFinalize, nil
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

result.Failed() indicates an error during the execution but it should not be treated as a fatal error for the call. Functions that use runMEVM need to differentiate between fatal errors and execution errors.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

}

if storageAccessTracer.hasStoredState {
Expand Down
2 changes: 1 addition & 1 deletion internal/ethapi/transaction_args.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func (args *TransactionArgs) setFeeDefaults(ctx context.Context, b Backend) erro
}
// Now attempt to fill in default value depending on whether London is active or not.
head := b.CurrentHeader()
if b.ChainConfig().IsLondon(head.Number) {
if b.ChainConfig().IsLondon(head.Number) && !args.IsConfidential {
lthibault marked this conversation as resolved.
Show resolved Hide resolved
// London is active, set maxPriorityFeePerGas and maxFeePerGas.
if err := args.setLondonFeeDefaults(ctx, head, b); err != nil {
return err
Expand Down
86 changes: 35 additions & 51 deletions suave/e2e/workflow_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ import (
"github.com/stretchr/testify/require"
)

func init() {
// use the gas estimation
sdk.SetDefaultGasLimit(0)
}

func TestIsConfidential(t *testing.T) {
// t.Fatal("not implemented")

Expand Down Expand Up @@ -1045,65 +1050,27 @@ func TestRelayBlockSubmissionContract(t *testing.T) {
require.True(t, ok)
}

func TestE2E_ForgeIntegration(t *testing.T) {
// This end-to-end test ensures that the precompile lifecycle expected in Forge works
func TestE2E_EstimateGas(t *testing.T) {
lthibault marked this conversation as resolved.
Show resolved Hide resolved
// This end-to-end test ensures that call and estimate gas works and it does not
// modify the confidential store
fr := newFramework(t, WithKettleAddress())
defer fr.Close()

rpcClient := fr.suethSrv.RPCNode()
ethClient := ethclient.NewClient(rpcClient)

chainIdRaw, err := ethClient.ChainID(context.Background())
require.NoError(t, err)

doCall := func(methodName string, args ...interface{}) []interface{} {
toAddr, ok := artifacts.SuaveMethods[methodName]
require.True(t, ok, fmt.Sprintf("suave method %s not found", methodName))

method := artifacts.SuaveAbi.Methods[methodName]

input, err := method.Inputs.Pack(args...)
require.NoError(t, err)

chainId := hexutil.Big(*chainIdRaw)

callArgs := ethapi.TransactionArgs{
To: &toAddr,
IsConfidential: true,
ChainID: &chainId,
Data: (*hexutil.Bytes)(&input),
}
var simResult hexutil.Bytes
err = rpcClient.Call(&simResult, "eth_call", setTxArgsDefaults(callArgs), "latest")
require.NoError(t, err)

if methodName == "confidentialRetrieve" {
// this method does not abi pack the output
return []interface{}{[]byte(simResult)}
}

result, err := method.Outputs.Unpack(simResult)
require.NoError(t, err)
return result
}

addrList := []common.Address{suave.AllowedPeekerAny}
recordRaw := doCall("newDataRecord", uint64(0), addrList, addrList, "default:v0:ethBundles")
newDataRecordInput := []interface{}{uint64(0), addrList, addrList, "default:v0:ethBundles"}

var record types.DataRecord
require.NoError(t, mapstructure.Decode(recordRaw[0], &record))
fr.callPrecompile("newDataRecord", newDataRecordInput)

recordsRaw := doCall("fetchDataRecords", uint64(0), "default:v0:ethBundles")
var bids []types.DataRecord
require.NoError(t, mapstructure.Decode(recordsRaw[0], &bids))
require.Len(t, bids, 1)
require.Equal(t, bids[0].Id, record.Id)
// fetch the records and validate that it is empty since a call should not modify the store
vals := fr.callPrecompile("fetchDataRecords", []interface{}{uint64(0), "default:v0:ethBundles"})
require.Len(t, vals[0], 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we certain that vals always has a nonzero length?

Copy link
Collaborator Author

@ferranbt ferranbt Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yup, enforced in callPrecompile. callPrecompile fails if it cannot decode the output of fetchDataRecords and fetchDataRecords always returns one item, the data record.


val := []byte{0x1, 0x2, 0x3}
doCall("confidentialStore", record.Id, "a", val)
// estimate the gas for the call should not modify the store either
gasLimit := fr.estimateGasPrecompile("newDataRecord", newDataRecordInput)
require.NotZero(t, gasLimit)

valRaw := doCall("confidentialRetrieve", record.Id, "a")
require.Equal(t, val, valRaw[0])
vals = fr.callPrecompile("fetchDataRecords", []interface{}{uint64(0), "default:v0:ethBundles"})
require.Len(t, vals[0], 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question about len(vals). Just want to make sure the test doesn't panic instead of failing cleanly.

}

func TestE2EPrecompile_Call(t *testing.T) {
Expand Down Expand Up @@ -1519,6 +1486,23 @@ func (f *framework) Close() {
f.suethSrv.Close()
}

func (f *framework) estimateGasPrecompile(name string, args []interface{}) uint64 {
input, err := artifacts.SuaveAbi.Methods[name].Inputs.Pack(args...)
require.NoError(f.t, err)

addr := artifacts.SuaveMethods[name]

var callResult hexutil.Uint64
err = f.suethSrv.RPCNode().Call(&callResult, "eth_estimateGas", ethapi.TransactionArgs{
To: &addr,
IsConfidential: true,
Data: (*hexutil.Bytes)(&input),
}, "latest")
requireNoRpcError(f.t, err)

return uint64(callResult)
}

func (f *framework) callPrecompile(name string, args []interface{}) []interface{} {
input, err := artifacts.SuaveAbi.Methods[name].Inputs.Pack(args...)
require.NoError(f.t, err)
Expand Down
25 changes: 24 additions & 1 deletion suave/sdk/sdk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,16 @@ import (
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/ethclient"
"github.com/ethereum/go-ethereum/internal/ethapi"
"github.com/ethereum/go-ethereum/rpc"
)

var defaultGasLimit = uint64(10000000)

func SetDefaultGasLimit(gasLimit uint64) {
defaultGasLimit = gasLimit
}

func DeployContract(bytecode []byte, client *Client) (*TransactionResult, error) {
txn := &types.LegacyTx{
Data: bytecode,
Expand Down Expand Up @@ -64,14 +71,30 @@ func (c *Contract) SendTransaction(method string, args []interface{}, confidenti
return nil, err
}

gasLimit := defaultGasLimit

if gasLimit == 0 {
var estimatedGasLimit hexutil.Uint64
err = c.client.rpc.Client().Call(&estimatedGasLimit, "eth_estimateGas", ethapi.TransactionArgs{
To: &c.addr,
IsConfidential: true,
ConfidentialInputs: (*hexutil.Bytes)(&confidentialDataBytes),
Data: (*hexutil.Bytes)(&calldata),
})
if err != nil {
return nil, err
}
gasLimit = uint64(estimatedGasLimit)
}

computeRequest, err := types.SignTx(types.NewTx(&types.ConfidentialComputeRequest{
ConfidentialComputeRecord: types.ConfidentialComputeRecord{
KettleAddress: c.client.kettleAddress,
Nonce: nonce,
To: &c.addr,
Value: nil,
GasPrice: gasPrice,
Gas: 1000000,
Gas: gasLimit,
Data: calldata,
},
ConfidentialInputs: confidentialDataBytes,
Expand Down
Loading