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.

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

Fix program recovery #3548

Merged
merged 1 commit into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion runtime/empty.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
18 changes: 14 additions & 4 deletions runtime/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
) (
Expand All @@ -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) {
Expand All @@ -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
}

Expand All @@ -593,6 +601,8 @@ func (e *interpreterEnvironment) recoverProgram(
return nil, nil
}

e.codesAndPrograms.setCode(location, newCode)

return program, elaboration
}

Expand Down
42 changes: 23 additions & 19 deletions runtime/ft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -969,27 +966,41 @@ 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

if !isFungibleTokenContract(program, contractsAddress) {
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

init() {
self.totalSupply = 0.0
}

access(all)
fun createEmptyVault(vaultType: Type): @{FungibleToken.Vault} {
ExampleToken.recoveryPanic("createEmptyVault")
}

access(all)
resource Vault: FungibleToken.Vault {

Expand All @@ -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
},
}

Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion runtime/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions runtime/tests/runtime_utils/testinterface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
Loading