Skip to content

Commit

Permalink
Merge pull request #3554 from onflow/bastian/fix-contract-update-error
Browse files Browse the repository at this point in the history 
Fix contract update error when old program has errors
  • Loading branch information
@turbolent
turbolent authored Aug 28, 2024
2 parents 50f44aa + 45f4ffa commit c18c5fa
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 15 deletions.
120 changes: 119 additions & 1 deletion runtime/contract_update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
. "github.com/onflow/cadence/runtime"
"github.com/onflow/cadence/runtime/common"
"github.com/onflow/cadence/runtime/interpreter"
"github.com/onflow/cadence/runtime/stdlib"
. "github.com/onflow/cadence/runtime/tests/runtime_utils"
. "github.com/onflow/cadence/runtime/tests/utils"
)
Expand Down Expand Up @@ -775,7 +776,7 @@ func TestRuntimeLegacyContractUpdate(t *testing.T) {
}
`

// Mock the deploy of the old 'Foo' contract
// Mock the deployment of the old 'Foo' contract
accountCodes[fooLocation] = []byte(fooContractV1)

// Programs are only valid during the transaction
Expand All @@ -794,7 +795,124 @@ func TestRuntimeLegacyContractUpdate(t *testing.T) {
},
)
require.NoError(t, err)
}

func TestRuntimeContractUpdateWithOldProgramError(t *testing.T) {
t.Parallel()

runtime := NewTestInterpreterRuntime()

accountCodes := map[common.Location][]byte{}
signerAccount := common.MustBytesToAddress([]byte{0x1})
fooLocation := common.AddressLocation{
Address: signerAccount,
Name: "Foo",
}
var checkGetAndSetProgram, getProgramCalled bool

programs := map[Location]*interpreter.Program{}
clearPrograms := func() {
for l := range programs {
delete(programs, l)
}
}

runtimeInterface := &TestRuntimeInterface{
OnGetCode: func(location Location) (bytes []byte, err error) {
return accountCodes[location], nil
},
Storage: NewTestLedger(nil, nil),
OnGetSigningAccounts: func() ([]Address, error) {
return []Address{signerAccount}, nil
},
OnResolveLocation: NewSingleIdentifierLocationResolver(t),
OnGetAccountContractCode: func(location common.AddressLocation) (code []byte, err error) {
return accountCodes[location], nil
},
OnGetAccountContractNames: func(_ Address) ([]string, error) {
return []string{"Foo"}, nil
},
OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error {
accountCodes[location] = code
return nil
},
OnEmitEvent: func(event cadence.Event) error {
return nil
},
OnDecodeArgument: func(b []byte, t cadence.Type) (value cadence.Value, err error) {
return json.Decode(nil, b)
},
OnGetAndSetProgram: func(
location Location,
load func() (*interpreter.Program, error),
) (
program *interpreter.Program,
err error,
) {
_, isTransactionLocation := location.(common.TransactionLocation)
if checkGetAndSetProgram && !isTransactionLocation {
require.Equal(t, location, fooLocation)
require.False(t, getProgramCalled)
}

var ok bool
program, ok = programs[location]
if ok {
return
}

program, err = load()

// NOTE: important: still set empty program,
// even if error occurred

programs[location] = program

return
},
}

nextTransactionLocation := NewTransactionLocationGenerator()

const fooContractV1 = `
pub contract Foo {
init() {}
pub fun hello() {}
}
`

const fooContractV2 = `
access(all) contract Foo {
init() {}
access(all) fun hello() {}
}
`

// Mock the deployment of the old 'Foo' contract
accountCodes[fooLocation] = []byte(fooContractV1)

// Programs are only valid during the transaction
clearPrograms()

// Update 'Foo' contract to Cadence 1.0 version

signerAccount = common.MustBytesToAddress([]byte{0x1})
err := runtime.ExecuteTransaction(
Script{
Source: UpdateTransaction("Foo", []byte(fooContractV2)),
},
Context{
Interface: runtimeInterface,
Location: nextTransactionLocation(),
},
)
RequireError(t, err)

var invalidContractDeploymentErr *stdlib.InvalidContractDeploymentError
require.ErrorAs(t, err, &invalidContractDeploymentErr)

require.ErrorContains(t,
err,
"pub contract Foo {\n | \t\t^^^\n\nerror: `pub` is no longer a valid access keyword",
)
}
61 changes: 47 additions & 14 deletions runtime/stdlib/account.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1470,6 +1470,32 @@ func newAccountContractsChangeFunction(
}
}

type OldProgramError struct {
Err error
Location common.Location
}

var _ errors.UserError = &OldProgramError{}
var _ errors.ParentError = &OldProgramError{}

func (e *OldProgramError) IsUserError() {}

func (e *OldProgramError) Error() string {
return "problem with old program"
}

func (e *OldProgramError) Unwrap() error {
return e.Err
}

func (e *OldProgramError) ChildErrors() []error {
return []error{e.Err}
}

func (e *OldProgramError) ImportLocation() common.Location {
return e.Location
}

func changeAccountContracts(
invocation interpreter.Invocation,
handler AccountContractAdditionAndNamesHandler,
Expand All @@ -1494,7 +1520,7 @@ func changeAccountContracts(
constructorArguments := invocation.Arguments[requiredArgumentCount:]
constructorArgumentTypes := invocation.ArgumentTypes[requiredArgumentCount:]

code, err := interpreter.ByteArrayValueToByteSlice(invocation.Interpreter, newCodeValue, locationRange)
newCode, err := interpreter.ByteArrayValueToByteSlice(invocation.Interpreter, newCodeValue, locationRange)
if err != nil {
panic(errors.NewDefaultUserError("add requires the second argument to be an array"))
}
Expand Down Expand Up @@ -1548,12 +1574,13 @@ func changeAccountContracts(
}

// Check the code
handleContractUpdateError := func(err error) {
handleContractUpdateError := func(err error, code []byte) {
if err == nil {
return
}

// Update the code for the error pretty printing
// Update the code for the error pretty printing.
// The code may be the new code, or the old code if this is an update.
// NOTE: only do this when an error occurs

handler.TemporarilyRecordCode(location, code)
Expand All @@ -1573,11 +1600,11 @@ func changeAccountContracts(
const getAndSetProgram = false

program, err := handler.ParseAndCheckProgram(
code,
newCode,
location,
getAndSetProgram,
)
handleContractUpdateError(err)
handleContractUpdateError(err, newCode)

// The code may declare exactly one contract or one contract interface.

Expand Down Expand Up @@ -1621,7 +1648,7 @@ func changeAccountContracts(
// Update the code for the error pretty printing
// NOTE: only do this when an error occurs

handler.TemporarilyRecordCode(location, code)
handler.TemporarilyRecordCode(location, newCode)

panic(errors.NewDefaultUserError(
"invalid %s: the code must declare exactly one contract or contract interface",
Expand All @@ -1636,7 +1663,7 @@ func changeAccountContracts(
// Update the code for the error pretty printing
// NOTE: only do this when an error occurs

handler.TemporarilyRecordCode(location, code)
handler.TemporarilyRecordCode(location, newCode)

panic(errors.NewDefaultUserError(
"invalid %s: the name argument must match the name of the declaration: got %q, expected %q",
Expand All @@ -1652,7 +1679,7 @@ func changeAccountContracts(

if isUpdate {
oldCode, err := handler.GetAccountContractCode(location)
handleContractUpdateError(err)
handleContractUpdateError(err, newCode)

memoryGauge := invocation.Interpreter.SharedState.Config.MemoryGauge
legacyUpgradeEnabled := invocation.Interpreter.SharedState.Config.LegacyContractUpgradeEnabled
Expand All @@ -1679,8 +1706,14 @@ func changeAccountContracts(
)
}

if !ignoreUpdatedProgramParserError(err) {
handleContractUpdateError(err)
if err != nil && !ignoreUpdatedProgramParserError(err) {
// NOTE: Errors are usually in the new program / new code,
// but here we failed for the old program / old code.
err = &OldProgramError{
Err: err,
Location: location,
}
handleContractUpdateError(err, oldCode)
}

var validator UpdateValidator
Expand All @@ -1706,14 +1739,14 @@ func changeAccountContracts(
validator = validator.WithTypeRemovalEnabled(contractUpdateTypeRemovalEnabled)

err = validator.Validate()
handleContractUpdateError(err)
handleContractUpdateError(err, newCode)
}

err = updateAccountContractCode(
handler,
location,
program,
code,
newCode,
contractType,
constructorArguments,
constructorArgumentTypes,
Expand All @@ -1725,7 +1758,7 @@ func changeAccountContracts(
// Update the code for the error pretty printing
// NOTE: only do this when an error occurs

handler.TemporarilyRecordCode(location, code)
handler.TemporarilyRecordCode(location, newCode)

panic(err)
}
Expand All @@ -1738,7 +1771,7 @@ func changeAccountContracts(
eventType = AccountContractAddedEventType
}

codeHashValue := CodeToHashValue(inter, code)
codeHashValue := CodeToHashValue(inter, newCode)

handler.EmitEvent(
inter,
Expand Down

0 comments on commit c18c5fa

Please sign in to comment.