From dd2fb5989dcc9d3ef1d2407fac773ce2ae8a8299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Thu, 22 Aug 2024 15:46:45 -0700 Subject: [PATCH] program recovery must return source code for error pretty printing --- runtime/empty.go | 2 +- runtime/environment.go | 18 +++++++-- runtime/ft_test.go | 42 +++++++++++--------- runtime/interface.go | 2 +- runtime/tests/runtime_utils/testinterface.go | 4 +- 5 files changed, 41 insertions(+), 27 deletions(-) diff --git a/runtime/empty.go b/runtime/empty.go index c8bbb4939a..a80db85745 100644 --- a/runtime/empty.go +++ b/runtime/empty.go @@ -235,6 +235,6 @@ func (EmptyRuntimeInterface) GenerateAccountID(_ common.Address) (uint64, error) panic("unexpected call to GenerateAccountID") } -func (EmptyRuntimeInterface) RecoverProgram(_ *ast.Program, _ common.Location) (*ast.Program, error) { +func (EmptyRuntimeInterface) RecoverProgram(_ *ast.Program, _ common.Location) ([]byte, error) { panic("unexpected call to RecoverProgram") } diff --git a/runtime/environment.go b/runtime/environment.go index 2ef2f432d7..2225f6fb69 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -556,7 +556,7 @@ func (e *interpreterEnvironment) parseAndCheckProgram( // recoverProgram parses and checks the given program with the old parser, // and recovers the elaboration from the old program. func (e *interpreterEnvironment) recoverProgram( - code []byte, + oldCode []byte, location common.Location, checkedImports importResolutionResults, ) ( @@ -568,7 +568,7 @@ func (e *interpreterEnvironment) recoverProgram( var err error reportMetric( func() { - program, err = old_parser.ParseProgram(e, code, old_parser.Config{}) + program, err = old_parser.ParseProgram(e, oldCode, old_parser.Config{}) }, e.runtimeInterface, func(metrics Metrics, duration time.Duration) { @@ -581,10 +581,18 @@ func (e *interpreterEnvironment) recoverProgram( // Recover elaboration from the old program + var newCode []byte errors.WrapPanic(func() { - program, err = e.runtimeInterface.RecoverProgram(program, location) + newCode, err = e.runtimeInterface.RecoverProgram(program, location) }) - if err != nil || program == nil { + if err != nil || newCode == nil { + return nil, nil + } + + // Parse and check the recovered program + + program, err = parser.ParseProgram(e, newCode, parser.Config{}) + if err != nil { return nil, nil } @@ -593,6 +601,8 @@ func (e *interpreterEnvironment) recoverProgram( return nil, nil } + e.codesAndPrograms.setCode(location, newCode) + return program, elaboration } diff --git a/runtime/ft_test.go b/runtime/ft_test.go index d962dcafcb..8282108b2b 100644 --- a/runtime/ft_test.go +++ b/runtime/ft_test.go @@ -30,7 +30,6 @@ import ( "github.com/onflow/cadence/runtime/ast" "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/interpreter" - "github.com/onflow/cadence/runtime/parser" "github.com/onflow/cadence/runtime/sema" . "github.com/onflow/cadence/runtime/tests/runtime_utils" "github.com/onflow/cadence/runtime/tests/utils" @@ -944,8 +943,6 @@ func TestRuntimeBrokenFungibleTokenRecovery(t *testing.T) { signerAccount := contractsAddress - var memoryGauge common.MemoryGauge - runtimeInterface := &TestRuntimeInterface{ OnGetCode: func(location Location) (bytes []byte, err error) { return accountCodes[location], nil @@ -969,7 +966,7 @@ func TestRuntimeBrokenFungibleTokenRecovery(t *testing.T) { OnDecodeArgument: func(b []byte, t cadence.Type) (value cadence.Value, err error) { return json.Decode(nil, b) }, - OnRecoverProgram: func(program *ast.Program, location common.Location) (*ast.Program, error) { + OnRecoverProgram: func(program *ast.Program, location common.Location) ([]byte, error) { // TODO: generalize @@ -977,12 +974,21 @@ func TestRuntimeBrokenFungibleTokenRecovery(t *testing.T) { return nil, nil } - code := ` + return []byte(` import FungibleToken from 0x1 access(all) contract ExampleToken: FungibleToken { + access(self) + view fun recoveryPanic(_ functionName: String): Never { + return panic( + "Contract ExampleToken is no longer functional. " + .concat("A version of the contract has been recovered to allow access to the fields declared in the FT standard. ") + .concat(functionName).concat(" is not available in recovered program.") + ) + } + access(all) var totalSupply: UFix64 @@ -990,6 +996,11 @@ func TestRuntimeBrokenFungibleTokenRecovery(t *testing.T) { self.totalSupply = 0.0 } + access(all) + fun createEmptyVault(vaultType: Type): @{FungibleToken.Vault} { + ExampleToken.recoveryPanic("createEmptyVault") + } + access(all) resource Vault: FungibleToken.Vault { @@ -1002,33 +1013,25 @@ func TestRuntimeBrokenFungibleTokenRecovery(t *testing.T) { access(FungibleToken.Withdraw) fun withdraw(amount: UFix64): @{FungibleToken.Vault} { - panic("withdraw is not implemented") + ExampleToken.recoveryPanic("Vault.withdraw") } access(all) view fun isAvailableToWithdraw(amount: UFix64): Bool { - panic("isAvailableToWithdraw is not implemented") + ExampleToken.recoveryPanic("Vault.isAvailableToWithdraw") } access(all) fun deposit(from: @{FungibleToken.Vault}) { - panic("deposit is not implemented") + ExampleToken.recoveryPanic("Vault.deposit") } access(all) fun createEmptyVault(): @{FungibleToken.Vault} { - panic("createEmptyVault is not implemented") + ExampleToken.recoveryPanic("Vault.createEmptyVault") } } - - access(all) - fun createEmptyVault(vaultType: Type): @{FungibleToken.Vault} { - panic("createEmptyVault is not implemented") - } } - ` - - // TODO: meter - return parser.ParseProgram(memoryGauge, []byte(code), parser.Config{}) + `), nil }, } @@ -1220,7 +1223,8 @@ func TestRuntimeBrokenFungibleTokenRecovery(t *testing.T) { }, ) utils.RequireError(t, err) - require.ErrorContains(t, err, "panic: withdraw is not implemented") + require.ErrorContains(t, err, "Vault.withdraw is not available in recovered program") + t.Log(err.Error()) // Send another transaction that loads the broken ExampleToken contract and the broken ExampleToken.Vault. // Accessing the broken ExampleToken contract value and ExampleToken.Vault resource again should not cause a panic. diff --git a/runtime/interface.go b/runtime/interface.go index ee78146bb4..13c16789d3 100644 --- a/runtime/interface.go +++ b/runtime/interface.go @@ -144,7 +144,7 @@ type Interface interface { ) // GenerateAccountID generates a new, *non-zero*, unique ID for the given account. GenerateAccountID(address common.Address) (uint64, error) - RecoverProgram(program *ast.Program, location common.Location) (*ast.Program, error) + RecoverProgram(program *ast.Program, location common.Location) ([]byte, error) } type MeterInterface interface { diff --git a/runtime/tests/runtime_utils/testinterface.go b/runtime/tests/runtime_utils/testinterface.go index c79e179b64..a73f9199a7 100644 --- a/runtime/tests/runtime_utils/testinterface.go +++ b/runtime/tests/runtime_utils/testinterface.go @@ -121,7 +121,7 @@ type TestRuntimeInterface struct { OnMemoryUsed func() (uint64, error) OnInteractionUsed func() (uint64, error) OnGenerateAccountID func(address common.Address) (uint64, error) - OnRecoverProgram func(program *ast.Program, location common.Location) (*ast.Program, error) + OnRecoverProgram func(program *ast.Program, location common.Location) ([]byte, error) lastUUID uint64 accountIDs map[common.Address]uint64 @@ -608,7 +608,7 @@ func (i *TestRuntimeInterface) InvalidateUpdatedPrograms() { } } -func (i *TestRuntimeInterface) RecoverProgram(program *ast.Program, location common.Location) (*ast.Program, error) { +func (i *TestRuntimeInterface) RecoverProgram(program *ast.Program, location common.Location) ([]byte, error) { if i.OnRecoverProgram == nil { return nil, nil }