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

[EVM] Contract deploy change return value #5606

Merged
merged 35 commits into from
Apr 25, 2024
Merged

[EVM] Contract deploy change return value #5606

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
fc3e408
update contract deploy to return result
Mar 29, 2024
a5c5217
update account type
Mar 29, 2024
794de19
update comments
Mar 29, 2024
0aa5fa7
return result on deploy
Mar 29, 2024
0ffb4d6
update deploy handler to return address as result
Mar 29, 2024
b7a5bfa
account fix comment
Mar 29, 2024
9e81add
update the test with new result type
Mar 29, 2024
dc70221
handler tests fix
Mar 29, 2024
5db7042
panic on error only
Apr 2, 2024
9745641
assign result in emulator
Apr 2, 2024
5ddbfb2
overwrite deployed address for call
Apr 2, 2024
9d3d855
update evm test
Apr 2, 2024
8819c86
handler test update
Apr 2, 2024
2c3a9ae
fix evm test assertion
Apr 2, 2024
04cf4c8
fix contract test
Apr 2, 2024
82520ec
update comment
Apr 2, 2024
a1e51e9
add contract address field
Apr 11, 2024
48445ed
update name
Apr 11, 2024
5bfcb86
add deployed contract address
Apr 11, 2024
d5f9a84
use deployed contract address field
Apr 11, 2024
1269e91
add init contract address
Apr 11, 2024
3fb8536
result to evm result
Apr 11, 2024
611156d
test the evm deploy result
Apr 11, 2024
bc60205
specify deployment contract
Apr 11, 2024
e0ff8fa
add empty address check for non-deploy
Apr 11, 2024
46bef61
fix deploy address test
Apr 11, 2024
403b9af
change result to be optional
Apr 22, 2024
904ee8c
fix name change
Apr 23, 2024
167bee1
change the result type for deployed contract to be pointer, so it can…
Apr 23, 2024
4a7b100
make the result deployed address pointer and optional
Apr 23, 2024
49b7f43
set optional if contract address not present
Apr 23, 2024
cc1e6a2
test changes with optional
Apr 23, 2024
0a0b559
additional check for contract addr
Apr 23, 2024
7be61a9
Merge branch 'master' into gregor/evm/deploy-result
devbugging Apr 23, 2024
7458cfe
Merge branch 'master' into gregor/evm/deploy-result
devbugging Apr 24, 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
5 changes: 4 additions & 1 deletion fvm/evm/emulator/emulator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ func (proc *procedure) deployAt(

proc.state.SetCode(addr, ret)
res.DeployedContractAddress = to
res.ReturnedValue = to[:] // assign address to return value since it's the only returned value
Copy link
Collaborator

@m-Peter m-Peter Apr 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have one concern about making this change here. Right now, a contract deployment results in the following evm.TransactionExecuted event payload:

evm.TransactionExecuted(
    blockHeight: 3,
    blockHash: "0xc9a60a5503928c637fbed960b8224e8786f147d1e9a4b6245527f50df0e7fbad",
    transactionHash: "0xe414f90fea2aebd75e8c0b3b6a4a0c9928e86c16ea724343d884f40bfe2c4c6b",
    transaction: "f9015880808301e8488080b901086060604052341561000f57600080fd5b60eb8061001d6000396000f300606060405260043610603f576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff168063c6888fa1146044575b600080fd5b3415604e57600080fd5b606260048080359060200190919050506078565b6040518082815260200191505060405180910390f35b60007f24abdb5865df5079dcc5ac590ff6f01d5c16edbc5fab4e195d9febd1114503da600783026040518082815260200191505060405180910390a16007820290509190505600a165627a7a7230582040383f19d9f65246752244189b02f56e8d0980ed44e7a56c0b200458caad20bb002982052fa09c05a7389284dc02b356ec7dee8a023c5efd3a9d844fa3c481882684b0640866a057e96d0a71a857ed509bb2b7333e78b2408574b8cc7f51238f25c58812662653",
    failed: false,
    vmError: "",
    transactionType: 0,
    gasConsumed: 103856,
    deployedContractAddress: "0x99466ED2E37B892A2Ee3E9CD55a98b68f5735db2",
    returnedValue: "606060405260043610603f576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff168063c6888fa1146044575b600080fd5b3415604e57600080fd5b606260048080359060200190919050506078565b6040518082815260200191505060405180910390f35b60007f24abdb5865df5079dcc5ac590ff6f01d5c16edbc5fab4e195d9febd1114503da600783026040518082815260200191505060405180910390a16007820290509190505600a165627a7a7230582040383f19d9f65246752244189b02f56e8d0980ed44e7a56c0b200458caad20bb0029",
    logs: ""
)

Where returnedValue holds the executed byte-code, basically what should be returned by eth_getCode for the deployed contract address.

With res.ReturnedValue = to[:], we will basically return the deployed contract address for both deployedContractAddress & returnedValue.

This results in less data being available in the emitted event. For example, the Gateway can use the current event payload and index the executed byte-code for each address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is true and I would say the only way to resolve this would to add the deployed address value to the EVM.Result type, the other way is to revert to overwrite the ReturnedValue in the contract handler no in emulator. cc @ramtinms

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@m-Peter I added deployed contract address as the new field on the result type

Copy link
Member

Choose a reason for hiding this comment

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

Left a question in the FLIP: onflow/flips#225 (comment)

return res, proc.commitAndFinalize()
}

Expand Down Expand Up @@ -442,9 +443,11 @@ func (proc *procedure) run(
res.GasConsumed = execResult.UsedGas
if !execResult.Failed() { // collect vm errors
res.ReturnedValue = execResult.ReturnData
// If the transaction created a contract, store the creation address in the receipt.
// If the transaction created a contract, store the creation address in the receipt,
// and overwrite the return value with the contract address.
if msg.To == nil {
res.DeployedContractAddress = types.NewAddress(gethCrypto.CreateAddress(msg.From, msg.Nonce))
res.ReturnedValue = res.DeployedContractAddress[:]
}
// replace tx index and tx hash
res.Logs = proc.state.Logs(
Expand Down
12 changes: 9 additions & 3 deletions fvm/evm/evm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ func TestCadenceOwnedAccountFunctionalities(t *testing.T) {
import FlowToken from %s

access(all)
fun main(): [UInt8; 20] {
fun main(): EVM.Result {
let admin = getAuthAccount(%s)
.borrow<&FlowToken.Administrator>(from: /storage/flowTokenAdmin)!
let minter <- admin.createNewMinter(allowedAmount: 2.34)
Expand All @@ -613,13 +613,13 @@ func TestCadenceOwnedAccountFunctionalities(t *testing.T) {
let cadenceOwnedAccount <- EVM.createCadenceOwnedAccount()
cadenceOwnedAccount.deposit(from: <-vault)

let address = cadenceOwnedAccount.deploy(
let res = cadenceOwnedAccount.deploy(
code: [],
gasLimit: 53000,
value: EVM.Balance(attoflow: 1230000000000000000)
)
destroy cadenceOwnedAccount
return address.bytes
return res
}
`,
sc.EVMContract.Address.HexWithPrefix(),
Expand All @@ -635,6 +635,12 @@ func TestCadenceOwnedAccountFunctionalities(t *testing.T) {
snapshot)
require.NoError(t, err)
require.NoError(t, output.Err)

res, err := stdlib.ResultSummaryFromEVMResultValue(output.Value)
require.NoError(t, err)
require.Equal(t, types.StatusSuccessful, res.Status)
require.Equal(t, types.ErrCodeNoError, res.ErrorCode)
require.NotNil(t, res.ReturnedValue)
})
})
}
Expand Down
9 changes: 5 additions & 4 deletions fvm/evm/handler/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,12 +556,13 @@ func (a *Account) transfer(to types.Address, balance types.Balance) (*types.Resu
}

// Deploy deploys a contract to the EVM environment
// the new deployed contract would be at the returned address and
// the new deployed contract would be at the returned address
// contained in the result summary as data and
// the contract data is not controlled by the caller accounts
func (a *Account) Deploy(code types.Code, gaslimit types.GasLimit, balance types.Balance) types.Address {
func (a *Account) Deploy(code types.Code, gaslimit types.GasLimit, balance types.Balance) *types.ResultSummary {
res, err := a.deploy(code, gaslimit, balance)
panicOnErrorOrInvalidOrFailedState(res, err)
return types.Address(res.DeployedContractAddress)
panicOnError(err)
return res.ResultSummary()
}

func (a *Account) deploy(code types.Code, gaslimit types.GasLimit, balance types.Balance) (*types.Result, error) {
Expand Down
11 changes: 8 additions & 3 deletions fvm/evm/handler/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,10 @@ func TestHandler_COA(t *testing.T) {
require.Equal(t, bal, foa.Balance())

testContract := testutils.GetStorageTestContract(t)
addr := foa.Deploy(testContract.ByteCode, math.MaxUint64, types.NewBalanceFromUFix64(0))
result := foa.Deploy(testContract.ByteCode, math.MaxUint64, types.NewBalanceFromUFix64(0))
addr := result.DeployedContractAddress
require.NotNil(t, addr)
assert.Equal(t, addr.Bytes(), []byte(result.ReturnedValue))

num := big.NewInt(22)
_ = foa.Call(
Expand All @@ -606,7 +608,7 @@ func TestHandler_COA(t *testing.T) {
math.MaxUint64,
types.NewBalanceFromUFix64(0))

require.Equal(t, num, new(big.Int).SetBytes(res.ReturnedValue))
require.Equal(t, num, res.ReturnedValue.AsBigInt())
})
})
})
Expand Down Expand Up @@ -655,8 +657,11 @@ func TestHandler_COA(t *testing.T) {
foa.Deposit(vault)

testContract := testutils.GetStorageTestContract(t)
addr := foa.Deploy(testContract.ByteCode, math.MaxUint64, types.EmptyBalance)
result := foa.Deploy(testContract.ByteCode, math.MaxUint64, types.EmptyBalance)
addr := result.DeployedContractAddress
require.NotNil(t, addr)
require.Equal(t, types.StatusSuccessful, result.Status)
require.Equal(t, types.ErrCodeNoError, result.ErrorCode)

ret := foa.Call(
addr,
Expand Down
10 changes: 5 additions & 5 deletions fvm/evm/stdlib/contract.cdc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,20 +255,20 @@ contract EVM {
}

/// Deploys a contract to the EVM environment.
/// Returns the address of the newly deployed contract
/// Returns the result which contains address of
/// the newly deployed contract
access(all)
fun deploy(
code: [UInt8],
gasLimit: UInt64,
value: Balance
): EVMAddress {
let addressBytes = InternalEVM.deploy(
): Result {
return InternalEVM.deploy(
from: self.addressBytes,
code: code,
gasLimit: gasLimit,
value: value.attoflow
)
return EVMAddress(bytes: addressBytes)
) as! Result
}

/// Calls a function with the given data.
Expand Down
30 changes: 27 additions & 3 deletions fvm/evm/stdlib/contract.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,29 @@ func ContractCode(flowTokenAddress flow.Address, evmAbiOnly bool) []byte {
}

const ContractName = "EVM"

const evmAddressTypeBytesFieldName = "bytes"

const evmAddressTypeQualifiedIdentifier = "EVM.EVMAddress"

const evmBalanceTypeQualifiedIdentifier = "EVM.Balance"

const evmResultTypeQualifiedIdentifier = "EVM.Result"

const evmStatusTypeQualifiedIdentifier = "EVM.Status"

const evmBlockTypeQualifiedIdentifier = "EVM.EVMBlock"

const abiEncodingByteSize = 32

var EVMTransactionBytesCadenceType = cadence.NewVariableSizedArrayType(cadence.TheUInt8Type)

var evmTransactionBytesType = sema.NewVariableSizedType(nil, sema.UInt8Type)

var evmAddressBytesType = sema.NewConstantSizedType(nil, sema.UInt8Type, types.AddressLength)

var evmAddressBytesStaticType = interpreter.ConvertSemaArrayTypeToStaticArrayType(nil, evmAddressBytesType)

var EVMAddressBytesCadenceType = cadence.NewConstantSizedArrayType(types.AddressLength, cadence.TheUInt8Type)

// abiEncodingError
Expand Down Expand Up @@ -264,19 +273,33 @@ func newInternalEVMTypeEncodeABIFunction(
}

var gethTypeString = gethABI.Type{T: gethABI.StringTy}

var gethTypeBool = gethABI.Type{T: gethABI.BoolTy}

var gethTypeUint8 = gethABI.Type{T: gethABI.UintTy, Size: 8}

var gethTypeUint16 = gethABI.Type{T: gethABI.UintTy, Size: 16}

var gethTypeUint32 = gethABI.Type{T: gethABI.UintTy, Size: 32}

var gethTypeUint64 = gethABI.Type{T: gethABI.UintTy, Size: 64}

var gethTypeUint128 = gethABI.Type{T: gethABI.UintTy, Size: 128}

var gethTypeUint256 = gethABI.Type{T: gethABI.UintTy, Size: 256}

var gethTypeInt8 = gethABI.Type{T: gethABI.IntTy, Size: 8}

var gethTypeInt16 = gethABI.Type{T: gethABI.IntTy, Size: 16}

var gethTypeInt32 = gethABI.Type{T: gethABI.IntTy, Size: 32}

var gethTypeInt64 = gethABI.Type{T: gethABI.IntTy, Size: 64}

var gethTypeInt128 = gethABI.Type{T: gethABI.IntTy, Size: 128}

var gethTypeInt256 = gethABI.Type{T: gethABI.IntTy, Size: 256}

var gethTypeAddress = gethABI.Type{Size: 20, T: gethABI.AddressTy}

func gethABIType(staticType interpreter.StaticType, evmAddressTypeID common.TypeID) (gethABI.Type, bool) {
Expand Down Expand Up @@ -1579,7 +1602,8 @@ var internalEVMTypeDeployFunctionType = &sema.FunctionType{
TypeAnnotation: sema.NewTypeAnnotation(sema.UIntType),
},
},
ReturnTypeAnnotation: sema.NewTypeAnnotation(evmAddressBytesType),
// Actually EVM.Result, but cannot refer to it here
ReturnTypeAnnotation: sema.NewTypeAnnotation(sema.AnyStructType),
}

func newInternalEVMTypeDeployFunction(
Expand Down Expand Up @@ -1639,9 +1663,9 @@ func newInternalEVMTypeDeployFunction(

const isAuthorized = true
account := handler.AccountByAddress(fromAddress, isAuthorized)
address := account.Deploy(code, gasLimit, amount)
result := account.Deploy(code, gasLimit, amount)

return EVMAddressToAddressBytesArrayValue(inter, address)
return NewResultValue(handler, gauge, inter, locationRange, result)
},
)
}
Expand Down
39 changes: 15 additions & 24 deletions fvm/evm/stdlib/contract_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ type testFlowAccount struct {
transfer func(address types.Address, balance types.Balance)
deposit func(vault *types.FLOWTokenVault)
withdraw func(balance types.Balance) *types.FLOWTokenVault
deploy func(code types.Code, limit types.GasLimit, balance types.Balance) types.Address
deploy func(code types.Code, limit types.GasLimit, balance types.Balance) *types.ResultSummary
call func(address types.Address, data types.Data, limit types.GasLimit, balance types.Balance) *types.ResultSummary
}

Expand Down Expand Up @@ -150,7 +150,7 @@ func (t *testFlowAccount) Withdraw(balance types.Balance) *types.FLOWTokenVault
return t.withdraw(balance)
}

func (t *testFlowAccount) Deploy(code types.Code, limit types.GasLimit, balance types.Balance) types.Address {
func (t *testFlowAccount) Deploy(code types.Code, limit types.GasLimit, balance types.Balance) *types.ResultSummary {
if t.deploy == nil {
panic("unexpected Deploy")
}
Expand Down Expand Up @@ -3589,20 +3589,24 @@ func TestCadenceOwnedAccountDeploy(t *testing.T) {
require.NoError(t, err)

handler := &testContractHandler{
flowTokenAddress: common.Address(contractsAddress),
evmContractAddress: common.Address(contractsAddress),
accountByAddress: func(fromAddress types.Address, isAuthorized bool) types.Account {
assert.Equal(t, types.Address{3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, fromAddress)
assert.True(t, isAuthorized)

return &testFlowAccount{
address: fromAddress,
deploy: func(code types.Code, limit types.GasLimit, balance types.Balance) types.Address {
deploy: func(code types.Code, limit types.GasLimit, balance types.Balance) *types.ResultSummary {
deployed = true
assert.Equal(t, types.Code{4, 5, 6}, code)
assert.Equal(t, types.GasLimit(9999), limit)
assert.Equal(t, types.NewBalanceFromUFix64(expectedBalance), balance)

return types.Address{4}
return &types.ResultSummary{
Status: types.StatusSuccessful,
DeployedContractAddress: types.Address{4},
ReturnedValue: types.Data{4},
}
},
}
},
Expand All @@ -3615,18 +3619,17 @@ func TestCadenceOwnedAccountDeploy(t *testing.T) {

script := []byte(`
import EVM from 0x1
import FlowToken from 0x1

access(all)
fun main(): [UInt8; 20] {
fun main(): [UInt8] {
let cadenceOwnedAccount <- EVM.createCadenceOwnedAccount()
let address = cadenceOwnedAccount.deploy(
let res = cadenceOwnedAccount.deploy(
code: [4, 5, 6],
gasLimit: 9999,
value: EVM.Balance(flow: 1230000000000000000)
)
destroy cadenceOwnedAccount
return address.bytes
return res.data
}
`)

Expand Down Expand Up @@ -3685,21 +3688,9 @@ func TestCadenceOwnedAccountDeploy(t *testing.T) {
)
require.NoError(t, err)

expected := cadence.NewArray([]cadence.Value{
cadence.UInt8(4), cadence.UInt8(0),
cadence.UInt8(0), cadence.UInt8(0),
cadence.UInt8(0), cadence.UInt8(0),
cadence.UInt8(0), cadence.UInt8(0),
cadence.UInt8(0), cadence.UInt8(0),
cadence.UInt8(0), cadence.UInt8(0),
cadence.UInt8(0), cadence.UInt8(0),
cadence.UInt8(0), cadence.UInt8(0),
cadence.UInt8(0), cadence.UInt8(0),
cadence.UInt8(0), cadence.UInt8(0),
}).WithType(cadence.NewConstantSizedArrayType(
types.AddressLength,
cadence.UInt8Type{},
))
expected := cadence.
NewArray([]cadence.Value{cadence.UInt8(4)}).
WithType(cadence.NewVariableSizedArrayType(cadence.UInt8Type{}))

require.Equal(t, expected, actual)

Expand Down
16 changes: 8 additions & 8 deletions fvm/evm/types/account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,19 @@ package types
// deploy contracts to the environment,
// or call methods on contracts without the need to sign a transaction.
type Account interface {
// Returns the address of this account
// Address returns the address of this account
Address() Address

// Returns the balance of this account
// Balance returns the balance of this account
Balance() Balance

// Returns the code of this account
// Code returns the code of this account
Code() Code

// Returns the code hash of this account
// CodeHash returns the code hash of this account
CodeHash() []byte

// Returns the nonce of this account
// Nonce returns the nonce of this account
Nonce() uint64

// Deposit deposits the token from the given vault into this account
Expand All @@ -39,10 +39,10 @@ type Account interface {
Transfer(to Address, balance Balance)

// Deploy deploys a contract to the environment
// the new deployed contract would be at the returned address and
// the contract data is not controlled by the COA
// the new deployed contract would be at the returned
// result address and the contract data is not controlled by the COA
// works only for COAs
Deploy(Code, GasLimit, Balance) Address
Deploy(Code, GasLimit, Balance) *ResultSummary

// Call calls a smart contract function with the given data.
// The gas usage is limited by the given gas limit,
Expand Down
Loading