Merge pull request #2045 from onetechnical/onetechnical/relbeta2.5.4v2.5.4-beta

go-algorand 2.5.4-beta

11 files changed, 376 insertions, 28 deletions

diff --git a/buildnumber.dat b/buildnumber.dat
index 00750edc0..b8626c4cf 100644
--- a/buildnumber.dat
+++ b/buildnumber.dat
@@ -1 +1 @@
-3
+4
diff --git a/crypto/merkletrie/cache.go b/crypto/merkletrie/cache.go
index 02e3dac0b..e4dc387dc 100644
--- a/crypto/merkletrie/cache.go
+++ b/crypto/merkletrie/cache.go
@@ -109,8 +109,9 @@ func (mtc *merkleTrieCache) initialize(mt *Trie, committer Committer, memoryConf
 	mtc.targetPageFillFactor = memoryConfig.PageFillFactor
 	mtc.maxChildrenPagesThreshold = memoryConfig.MaxChildrenPagesThreshold
 	if mt.nextNodeID != storedNodeIdentifierBase {
-		// if the next node is going to be on a new page, no need to reload the last page.
-		if (int64(mtc.mt.nextNodeID) / mtc.nodesPerPage) == (int64(mtc.mt.nextNodeID-1) / mtc.nodesPerPage) {
+		// If the next node would reside on a page that already has a few entries in it, make sure to mark it for late loading.
+		// Otherwise, the next node is going to be the first node on this page, we don't need to reload that page ( since it doesn't exist! ).
+		if (int64(mtc.mt.nextNodeID) % mtc.nodesPerPage) > 0 {
 			mtc.deferedPageLoad = uint64(mtc.mt.nextNodeID) / uint64(mtc.nodesPerPage)
 		}
 	}
@@ -262,7 +263,7 @@ func (mtc *merkleTrieCache) loadPage(page uint64) (err error) {
 	}
 
 	// if we've just loaded a deferred page, no need to reload it during the commit.
-	if mtc.deferedPageLoad != page {
+	if mtc.deferedPageLoad == page {
 		mtc.deferedPageLoad = storedNodeIdentifierNull
 	}
 	return
diff --git a/crypto/merkletrie/cache_test.go b/crypto/merkletrie/cache_test.go
index f54354aa9..8b50d8ea7 100644
--- a/crypto/merkletrie/cache_test.go
+++ b/crypto/merkletrie/cache_test.go
@@ -454,3 +454,36 @@ func TestCachePagedOutTip(t *testing.T) {
 	page = uint64(mt1.root) / uint64(memConfig.NodesCountPerPage)
 	require.NotNil(t, mt1.cache.pageToNIDsPtr[page])
 }
+
+// TestCacheLoadingDeferedPage verifies that the loadPage
+// method correcly resets the mtc.deferedPageLoad on the correct page.
+func TestCacheLoadingDeferedPage(t *testing.T) {
+	var memoryCommitter1 InMemoryCommitter
+	mt1, _ := MakeTrie(&memoryCommitter1, defaultTestMemoryConfig)
+	// create 100000 hashes.
+	leafsCount := 100000
+	hashes := make([]crypto.Digest, leafsCount)
+	for i := 0; i < len(hashes); i++ {
+		hashes[i] = crypto.Hash([]byte{byte(i % 256), byte((i / 256) % 256), byte(i / 65536)})
+	}
+
+	for i := 0; i < len(hashes); i++ {
+		mt1.Add(hashes[i][:])
+	}
+	_, err := mt1.Commit()
+	require.NoError(t, err)
+
+	// verify that the cache doesn't reset the mtc.deferedPageLoad on loading a non-defered page.
+	dupMem := memoryCommitter1.Duplicate()
+	mt2, _ := MakeTrie(dupMem, defaultTestMemoryConfig)
+	lastPage := int64(mt2.nextNodeID) / defaultTestMemoryConfig.NodesCountPerPage
+	require.Equal(t, uint64(lastPage), mt2.cache.deferedPageLoad)
+	err = mt2.cache.loadPage(uint64(lastPage - 1))
+	require.NoError(t, err)
+	require.Equal(t, uint64(lastPage), mt2.cache.deferedPageLoad)
+
+	// verify that the cache does reset the mtc.deferedPageLoad on loading a defered page.
+	err = mt2.cache.loadPage(uint64(lastPage))
+	require.NoError(t, err)
+	require.Equal(t, uint64(0), mt2.cache.deferedPageLoad)
+}
diff --git a/ledger/accountdb.go b/ledger/accountdb.go
index 1c37bb813..4a1640394 100644
--- a/ledger/accountdb.go
+++ b/ledger/accountdb.go
@@ -1341,7 +1341,7 @@ func makeMerkleCommitter(tx *sql.Tx, staging bool) (mc *merkleCommitter, err err
 	return mc, nil
 }
 
-// StorePage stores a single page in an in-memory persistence.
+// StorePage is the merkletrie.Committer interface implementation, stores a single page in a sqllite database table.
 func (mc *merkleCommitter) StorePage(page uint64, content []byte) error {
 	if len(content) == 0 {
 		_, err := mc.deleteStmt.Exec(page)
@@ -1351,7 +1351,7 @@ func (mc *merkleCommitter) StorePage(page uint64, content []byte) error {
 	return err
 }
 
-// LoadPage load a single page from an in-memory persistence.
+// LoadPage is the merkletrie.Committer interface implementation, load a single page from a sqllite database table.
 func (mc *merkleCommitter) LoadPage(page uint64) (content []byte, err error) {
 	err = mc.selectStmt.QueryRow(page).Scan(&content)
 	if err == sql.ErrNoRows {
diff --git a/ledger/appcow.go b/ledger/appcow.go
index aa0519def..0286bb612 100644
--- a/ledger/appcow.go
+++ b/ledger/appcow.go
@@ -25,6 +25,7 @@ import (
 	"github.com/algorand/go-algorand/data/transactions"
 	"github.com/algorand/go-algorand/data/transactions/logic"
 	"github.com/algorand/go-algorand/ledger/apply"
+	"github.com/algorand/go-algorand/ledger/ledgercore"
 	"github.com/algorand/go-algorand/protocol"
 )
 
@@ -419,7 +420,7 @@ func (cb *roundCowState) StatefulEval(params logic.EvalParams, aidx basics.AppIn
 	// Eval the program
 	pass, err = logic.EvalStateful(program, params)
 	if err != nil {
-		return false, basics.EvalDelta{}, err
+		return false, basics.EvalDelta{}, ledgercore.LogicEvalError{Err: err}
 	}
 
 	// If program passed, build our eval delta, and commit to state changes
@@ -566,16 +567,18 @@ func applyStorageDelta(data basics.AccountData, aapp storagePtr, store *storageD
 		case deallocAction:
 			delete(owned, aapp.aidx)
 		case allocAction, remainAllocAction:
-			// TODO verify this assertion
 			// note: these should always exist because they were
-			// at least preceded by a call to PutWithCreatable?
+			// at least preceded by a call to PutWithCreatable
 			params, ok := owned[aapp.aidx]
 			if !ok {
 				return basics.AccountData{}, fmt.Errorf("could not find existing params for %v", aapp.aidx)
 			}
 			params = params.Clone()
-			if store.action == allocAction {
-				// TODO does this ever accidentally clobber?
+			if (store.action == allocAction && len(store.kvCow) > 0) ||
+				(store.action == remainAllocAction && params.GlobalState == nil) {
+				// allocate KeyValue for
+				// 1) app creation and global write in the same app call
+				// 2) global state writing into empty global state
 				params.GlobalState = make(basics.TealKeyValue)
 			}
 			// note: if this is an allocAction, there will be no
@@ -602,7 +605,6 @@ func applyStorageDelta(data basics.AccountData, aapp storagePtr, store *storageD
 		case deallocAction:
 			delete(owned, aapp.aidx)
 		case allocAction, remainAllocAction:
-			// TODO verify this assertion
 			// note: these should always exist because they were
 			// at least preceded by a call to Put?
 			states, ok := owned[aapp.aidx]
@@ -610,8 +612,11 @@ func applyStorageDelta(data basics.AccountData, aapp storagePtr, store *storageD
 				return basics.AccountData{}, fmt.Errorf("could not find existing states for %v", aapp.aidx)
 			}
 			states = states.Clone()
-			if store.action == allocAction {
-				// TODO does this ever accidentally clobber?
+			if (store.action == allocAction && len(store.kvCow) > 0) ||
+				(store.action == remainAllocAction && states.KeyValue == nil) {
+				// allocate KeyValue for
+				// 1) opting in and local state write in the same app call
+				// 2) local state writing into empty local state (opted in)
 				states.KeyValue = make(basics.TealKeyValue)
 			}
 			// note: if this is an allocAction, there will be no
diff --git a/ledger/applications_test.go b/ledger/applications_test.go
index 8e4358f76..0e827940e 100644
--- a/ledger/applications_test.go
+++ b/ledger/applications_test.go
@@ -18,14 +18,19 @@ package ledger
 
 import (
 	"crypto/rand"
+	"encoding/hex"
 	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/algorand/go-algorand/config"
 	"github.com/algorand/go-algorand/crypto"
 	"github.com/algorand/go-algorand/data/basics"
 	"github.com/algorand/go-algorand/data/transactions"
+	"github.com/algorand/go-algorand/data/transactions/logic"
+	"github.com/algorand/go-algorand/logging"
+	"github.com/algorand/go-algorand/protocol"
 )
 
 func getRandomAddress(a *require.Assertions) basics.Address {
@@ -332,3 +337,233 @@ func TestLogicLedgerDelKey(t *testing.T) {
 	err = l.DelLocal(addr1, "lkey")
 	a.NoError(err)
 }
+
+// test ensures that
+// 1) app's GlobalState and local state's KeyValue are stored in the same way
+// before and after application code refactoring
+// 2) writing into empty (opted-in) local state's KeyValue works after reloading
+// Hardcoded values are from commit 9a0b439 (pre app refactor commit)
+func TestAppAccountDataStorage(t *testing.T) {
+	a := require.New(t)
+	source := `#pragma version 2
+// do not write local key on opt in or on app create
+txn ApplicationID
+int 0
+==
+bnz success
+txn OnCompletion
+int NoOp
+==
+bnz writetostate
+txn OnCompletion
+int OptIn
+==
+bnz checkargs
+int 0
+return
+checkargs:
+// if no args the success
+// otherwise write data
+txn NumAppArgs
+int 0
+==
+bnz success
+// write local or global key depending on arg1
+writetostate:
+txna ApplicationArgs 0
+byte "local"
+==
+bnz writelocal
+txna ApplicationArgs 0
+byte "global"
+==
+bnz writeglobal
+int 0
+return
+writelocal:
+int 0
+byte "lk"
+byte "local"
+app_local_put
+b success
+writeglobal:
+byte "gk"
+byte "global"
+app_global_put
+success:
+int 1
+return`
+
+	ops, err := logic.AssembleString(source)
+	a.NoError(err)
+	a.Greater(len(ops.Program), 1)
+	program := ops.Program
+
+	proto := config.Consensus[protocol.ConsensusCurrentVersion]
+	genesisInitState, initKeys := testGenerateInitState(t, protocol.ConsensusCurrentVersion)
+
+	creator, err := basics.UnmarshalChecksumAddress("3LN5DBFC2UTPD265LQDP3LMTLGZCQ5M3JV7XTVTGRH5CKSVNQVDFPN6FG4")
+	a.NoError(err)
+	userOptin, err := basics.UnmarshalChecksumAddress("6S6UMUQ4462XRGNON5GKBHW55RUJGJ5INIRDFVFD6KSPHGWGRKPC6RK2O4")
+	a.NoError(err)
+	userLocal, err := basics.UnmarshalChecksumAddress("UL5C6SRVLOROSB5FGAE6TY34VXPXVR7GNIELUB3DD5KTA4VT6JGOZ6WFAY")
+	a.NoError(err)
+	userLocal2, err := basics.UnmarshalChecksumAddress("XNOGOJECWDOMVENCDJHNMOYVV7PIVIJXRWTSZUA3GSKYTVXH3VVGOXP7CU")
+	a.NoError(err)
+
+	a.Contains(genesisInitState.Accounts, creator)
+	a.Contains(genesisInitState.Accounts, userOptin)
+	a.Contains(genesisInitState.Accounts, userLocal)
+	a.Contains(genesisInitState.Accounts, userLocal2)
+
+	expectedCreator, err := hex.DecodeString("84a4616c676fce009d2290a461707070810184a6617070726f76c45602200200012604056c6f63616c06676c6f62616c026c6b02676b3118221240003331192212400010311923124000022243311b221240001c361a00281240000a361a0029124000092243222a28664200032b29672343a6636c65617270c40102a46773636881a36e627304a46c73636881a36e627301a36f6e6c01a47473636881a36e627304")
+	a.NoError(err)
+	expectedUserOptIn, err := hex.DecodeString("84a4616c676fce00a02fd0a46170706c810181a46873636881a36e627301a36f6e6c01a47473636881a36e627301")
+	a.NoError(err)
+	expectedUserLocal, err := hex.DecodeString("84a4616c676fce00a33540a46170706c810182a46873636881a36e627301a3746b7681a26c6b82a27462a56c6f63616ca2747401a36f6e6c01a47473636881a36e627301")
+	a.NoError(err)
+
+	cfg := config.GetDefaultLocal()
+	l, err := OpenLedger(logging.Base(), "TestAppAccountData", true, genesisInitState, cfg)
+	a.NoError(err)
+	defer l.Close()
+
+	txHeader := transactions.Header{
+		Sender:      creator,
+		Fee:         basics.MicroAlgos{Raw: proto.MinTxnFee * 2},
+		FirstValid:  l.Latest() + 1,
+		LastValid:   l.Latest() + 10,
+		GenesisID:   t.Name(),
+		GenesisHash: genesisInitState.GenesisHash,
+	}
+
+	// create application
+	approvalProgram := program
+	clearStateProgram := []byte("\x02") // empty
+	appCreateFields := transactions.ApplicationCallTxnFields{
+		ApprovalProgram:   approvalProgram,
+		ClearStateProgram: clearStateProgram,
+		GlobalStateSchema: basics.StateSchema{NumByteSlice: 4},
+		LocalStateSchema:  basics.StateSchema{NumByteSlice: 1},
+	}
+	appCreate := transactions.Transaction{
+		Type:                     protocol.ApplicationCallTx,
+		Header:                   txHeader,
+		ApplicationCallTxnFields: appCreateFields,
+	}
+	err = l.appendUnvalidatedTx(t, genesisInitState.Accounts, initKeys, appCreate, transactions.ApplyData{})
+	a.NoError(err)
+
+	appIdx := basics.AppIndex(1) // first tnx => idx = 1
+
+	// opt-in, do no write
+	txHeader.Sender = userOptin
+	appCallFields := transactions.ApplicationCallTxnFields{
+		OnCompletion:  transactions.OptInOC,
+		ApplicationID: appIdx,
+	}
+	appCall := transactions.Transaction{
+		Type:                     protocol.ApplicationCallTx,
+		Header:                   txHeader,
+		ApplicationCallTxnFields: appCallFields,
+	}
+	err = l.appendUnvalidatedTx(t, genesisInitState.Accounts, initKeys, appCall, transactions.ApplyData{})
+	a.NoError(err)
+
+	// opt-in + write
+	txHeader.Sender = userLocal
+	appCall = transactions.Transaction{
+		Type:                     protocol.ApplicationCallTx,
+		Header:                   txHeader,
+		ApplicationCallTxnFields: appCallFields,
+	}
+	err = l.appendUnvalidatedTx(t, genesisInitState.Accounts, initKeys, appCall, transactions.ApplyData{})
+	a.NoError(err)
+
+	// save data into DB and write into local state
+	l.accts.accountsWriting.Add(1)
+	l.accts.commitRound(3, 0, 0)
+	l.reloadLedger()
+
+	appCallFields = transactions.ApplicationCallTxnFields{
+		OnCompletion:    0,
+		ApplicationID:   appIdx,
+		ApplicationArgs: [][]byte{[]byte("local")},
+	}
+	appCall = transactions.Transaction{
+		Type:                     protocol.ApplicationCallTx,
+		Header:                   txHeader,
+		ApplicationCallTxnFields: appCallFields,
+	}
+	err = l.appendUnvalidatedTx(t, genesisInitState.Accounts, initKeys, appCall,
+		transactions.ApplyData{EvalDelta: basics.EvalDelta{
+			LocalDeltas: map[uint64]basics.StateDelta{0: {"lk": basics.ValueDelta{Action: basics.SetBytesAction, Bytes: "local"}}}},
+		})
+	a.NoError(err)
+
+	// save data into DB
+	l.accts.accountsWriting.Add(1)
+	l.accts.commitRound(1, 3, 0)
+	l.reloadLedger()
+
+	// dump accounts
+	var rowid int64
+	var dbRound basics.Round
+	var buf []byte
+	err = l.accts.accountsq.lookupStmt.QueryRow(creator[:]).Scan(&rowid, &dbRound, &buf)
+	a.NoError(err)
+	a.Equal(expectedCreator, buf)
+
+	err = l.accts.accountsq.lookupStmt.QueryRow(userOptin[:]).Scan(&rowid, &dbRound, &buf)
+	a.NoError(err)
+	a.Equal(expectedUserOptIn, buf)
+	pad, err := l.accts.accountsq.lookup(userOptin)
+	a.Nil(pad.accountData.AppLocalStates[appIdx].KeyValue)
+	ad, err := l.Lookup(dbRound, userOptin)
+	a.Nil(ad.AppLocalStates[appIdx].KeyValue)
+
+	err = l.accts.accountsq.lookupStmt.QueryRow(userLocal[:]).Scan(&rowid, &dbRound, &buf)
+	a.NoError(err)
+	a.Equal(expectedUserLocal, buf)
+
+	ad, err = l.Lookup(dbRound, userLocal)
+	a.NoError(err)
+	a.Equal("local", ad.AppLocalStates[appIdx].KeyValue["lk"].Bytes)
+
+	// ensure writing into empty global state works as well
+	l.reloadLedger()
+	txHeader.Sender = creator
+	appCallFields = transactions.ApplicationCallTxnFields{
+		OnCompletion:    0,
+		ApplicationID:   appIdx,
+		ApplicationArgs: [][]byte{[]byte("global")},
+	}
+	appCall = transactions.Transaction{
+		Type:                     protocol.ApplicationCallTx,
+		Header:                   txHeader,
+		ApplicationCallTxnFields: appCallFields,
+	}
+	err = l.appendUnvalidatedTx(t, genesisInitState.Accounts, initKeys, appCall,
+		transactions.ApplyData{EvalDelta: basics.EvalDelta{
+			GlobalDelta: basics.StateDelta{"gk": basics.ValueDelta{Action: basics.SetBytesAction, Bytes: "global"}}},
+		})
+	a.NoError(err)
+
+	// opt-in + write by during opt-in
+	txHeader.Sender = userLocal2
+	appCallFields = transactions.ApplicationCallTxnFields{
+		OnCompletion:    transactions.OptInOC,
+		ApplicationID:   appIdx,
+		ApplicationArgs: [][]byte{[]byte("local")},
+	}
+	appCall = transactions.Transaction{
+		Type:                     protocol.ApplicationCallTx,
+		Header:                   txHeader,
+		ApplicationCallTxnFields: appCallFields,
+	}
+	err = l.appendUnvalidatedTx(t, genesisInitState.Accounts, initKeys, appCall,
+		transactions.ApplyData{EvalDelta: basics.EvalDelta{
+			LocalDeltas: map[uint64]basics.StateDelta{0: {"lk": basics.ValueDelta{Action: basics.SetBytesAction, Bytes: "local"}}}},
+		})
+	a.NoError(err)
+}
diff --git a/ledger/apply/application.go b/ledger/apply/application.go
index d593eed6e..e3665e1a7 100644
--- a/ledger/apply/application.go
+++ b/ledger/apply/application.go
@@ -22,6 +22,7 @@ import (
 	"github.com/algorand/go-algorand/data/basics"
 	"github.com/algorand/go-algorand/data/transactions"
 	"github.com/algorand/go-algorand/data/transactions/logic"
+	"github.com/algorand/go-algorand/ledger/ledgercore"
 )
 
 // Allocate the map of basics.AppParams if it is nil, and return a copy. We do *not*
@@ -368,14 +369,19 @@ func ApplicationCall(ac transactions.ApplicationCallTxnFields, header transactio
 		if exists {
 			pass, evalDelta, err := balances.StatefulEval(*evalParams, appIdx, params.ClearStateProgram)
 			if err != nil {
-				return err
+				// Fail on non-logic eval errors and ignore LogicEvalError errors
+				if _, ok := err.(ledgercore.LogicEvalError); !ok {
+					return err
+				}
 			}
 
-			// Fill in applyData, so that consumers don't have to implement a
-			// stateful TEAL interpreter to apply state changes
-			if pass {
-				// We will have applied any changes if and only if we passed
+			// We will have applied any changes if and only if we passed
+			if err == nil && pass {
+				// Fill in applyData, so that consumers don't have to implement a
+				// stateful TEAL interpreter to apply state changes
 				ad.EvalDelta = evalDelta
+			} else {
+				// Ignore logic eval errors and rejections from the ClearStateProgram
 			}
 		}
 
diff --git a/ledger/apply/application_test.go b/ledger/apply/application_test.go
index 65d74cae7..23f1a4909 100644
--- a/ledger/apply/application_test.go
+++ b/ledger/apply/application_test.go
@@ -28,6 +28,7 @@ import (
 	"github.com/algorand/go-algorand/data/basics"
 	"github.com/algorand/go-algorand/data/transactions"
 	"github.com/algorand/go-algorand/data/transactions/logic"
+	"github.com/algorand/go-algorand/ledger/ledgercore"
 	"github.com/algorand/go-algorand/protocol"
 )
 
@@ -110,6 +111,7 @@ type testBalances struct {
 	// logic evaluator control
 	pass  bool
 	delta basics.EvalDelta
+	err   error
 }
 
 type testBalancesPass struct {
@@ -182,7 +184,7 @@ func (b *testBalances) Deallocate(addr basics.Address, aidx basics.AppIndex, glo
 }
 
 func (b *testBalances) StatefulEval(params logic.EvalParams, aidx basics.AppIndex, program []byte) (passed bool, evalDelta basics.EvalDelta, err error) {
-	return b.pass, b.delta, nil
+	return b.pass, b.delta, b.err
 }
 
 func (b *testBalancesPass) Get(addr basics.Address, withPendingRewards bool) (basics.AccountData, error) {
@@ -665,7 +667,7 @@ func TestAppCallClearState(t *testing.T) {
 
 	ac := transactions.ApplicationCallTxnFields{
 		ApplicationID: appIdx,
-		OnCompletion:  transactions.CloseOutOC,
+		OnCompletion:  transactions.ClearStateOC,
 	}
 	params := basics.AppParams{
 		ApprovalProgram: []byte{1},
@@ -689,7 +691,7 @@ func TestAppCallClearState(t *testing.T) {
 	b.balances[sender] = basics.AccountData{}
 	err := ApplicationCall(ac, h, &b, ad, &ep, txnCounter)
 	a.Error(err)
-	a.Contains(err.Error(), "is not opted in to app")
+	a.Contains(err.Error(), "is not currently opted in to app")
 	a.Equal(0, b.put)
 	a.Equal(0, b.putWith)
 
@@ -741,8 +743,8 @@ func TestAppCallClearState(t *testing.T) {
 	b.appCreators[appIdx] = creator
 
 	// one put: to opt out
-	gd := basics.StateDelta{"uint": {Action: basics.SetUintAction, Uint: 1}}
-	b.delta = basics.EvalDelta{GlobalDelta: gd}
+	b.pass = false
+	b.delta = basics.EvalDelta{GlobalDelta: nil}
 	err = ApplicationCall(ac, h, &b, ad, &ep, txnCounter)
 	a.NoError(err)
 	a.Equal(1, b.put)
@@ -750,13 +752,46 @@ func TestAppCallClearState(t *testing.T) {
 	br = b.putBalances[sender]
 	a.Equal(0, len(br.AppLocalStates))
 	a.Equal(basics.StateSchema{}, br.TotalAppSchema)
-	a.Equal(gd, ad.EvalDelta.GlobalDelta)
+	a.Equal(basics.StateDelta(nil), ad.EvalDelta.GlobalDelta)
 
 	b.ResetWrites()
 
-	// check existing application with successful ClearStateProgram. two
-	// one to opt out, one deallocate
+	// check existing application with logic err ClearStateProgram.
+	// one to opt out, one deallocate, no error from ApplicationCall
 	b.pass = true
+	b.delta = basics.EvalDelta{GlobalDelta: nil}
+	b.err = ledgercore.LogicEvalError{Err: fmt.Errorf("test error")}
+	err = ApplicationCall(ac, h, &b, ad, &ep, txnCounter)
+	a.NoError(err)
+	a.Equal(1, b.put)
+	a.Equal(0, b.putWith)
+	br = b.putBalances[sender]
+	a.Equal(0, len(br.AppLocalStates))
+	a.Equal(basics.StateSchema{}, br.TotalAppSchema)
+	a.Equal(basics.StateDelta(nil), ad.EvalDelta.GlobalDelta)
+
+	b.ResetWrites()
+
+	// check existing application with non-logic err ClearStateProgram.
+	// ApplicationCall must fail
+	b.pass = true
+	b.delta = basics.EvalDelta{GlobalDelta: nil}
+	b.err = fmt.Errorf("test error")
+	err = ApplicationCall(ac, h, &b, ad, &ep, txnCounter)
+	a.Error(err)
+	br = b.putBalances[sender]
+	a.Equal(0, len(br.AppLocalStates))
+	a.Equal(basics.StateSchema{}, br.TotalAppSchema)
+	a.Equal(basics.StateDelta(nil), ad.EvalDelta.GlobalDelta)
+
+	b.ResetWrites()
+
+	// check existing application with successful ClearStateProgram.
+	// one to opt out, one deallocate, no error from ApplicationCall
+	b.pass = true
+	b.err = nil
+	gd := basics.StateDelta{"uint": {Action: basics.SetUintAction, Uint: 1}}
+	b.delta = basics.EvalDelta{GlobalDelta: gd}
 	err = ApplicationCall(ac, h, &b, ad, &ep, txnCounter)
 	a.NoError(err)
 	a.Equal(1, b.put)
diff --git a/ledger/ledgercore/error.go b/ledger/ledgercore/error.go
index ae5393441..65cbcff02 100644
--- a/ledger/ledgercore/error.go
+++ b/ledger/ledgercore/error.go
@@ -76,3 +76,13 @@ type ErrNoEntry struct {
 func (err ErrNoEntry) Error() string {
 	return fmt.Sprintf("ledger does not have entry %d (latest %d, committed %d)", err.Round, err.Latest, err.Committed)
 }
+
+// LogicEvalError indicates TEAL evaluation failure
+type LogicEvalError struct {
+	Err error
+}
+
+// Error satisfies builtin interface `error`
+func (err LogicEvalError) Error() string {
+	return fmt.Sprintf("logic eval error: %v", err.Err)
+}
diff --git a/node/node.go b/node/node.go
index 851afd8e9..29fff9316 100644
--- a/node/node.go
+++ b/node/node.go
@@ -410,6 +410,10 @@ func (node *AlgorandFullNode) Stop() {
 	defer func() {
 		node.mu.Unlock()
 		node.waitMonitoringRoutines()
+		// we want to shut down the compactCert last, since the oldKeyDeletionThread might depend on it when making the
+		// call to LatestSigsFromThisNode.
+		node.compactCert.Shutdown()
+		node.compactCert = nil
 	}()
 
 	node.net.ClearHandlers()
@@ -429,7 +433,6 @@ func (node *AlgorandFullNode) Stop() {
 	node.lowPriorityCryptoVerificationPool.Shutdown()
 	node.cryptoPool.Shutdown()
 	node.cancelCtx()
-	node.compactCert.Shutdown()
 	if node.indexer != nil {
 		node.indexer.Shutdown()
 	}
diff --git a/test/scripts/e2e_subs/e2e-app-simple.sh b/test/scripts/e2e_subs/e2e-app-simple.sh
index d67486c10..e770ee872 100755
--- a/test/scripts/e2e_subs/e2e-app-simple.sh
+++ b/test/scripts/e2e_subs/e2e-app-simple.sh
@@ -101,3 +101,23 @@ if [[ $RES != *"${EXPERROR}"* ]]; then
     date '+app-create-test FAIL clearing state twice should fail %Y%m%d_%H%M%S'
     false
 fi
+
+# Create an application with clear program always errs
+# Ensure clear still works
+APPID=$(${gcmd} app create --creator ${ACCOUNT} --approval-prog <(printf '#pragma version 2\nint 1') --clear-prog <(printf '#pragma version 2\nerr') --global-byteslices 0 --global-ints 0 --local-byteslices 0 --local-ints 0 | grep Created | awk '{ print $6 }')
+
+# Should succeed to opt in
+${gcmd} app optin --app-id $APPID --from $ACCOUNT
+
+# Succeed in clearing state for the app
+${gcmd} app clear --app-id $APPID --from $ACCOUNT
+
+# Create an application with clear program always fails
+# Ensure clear still works
+APPID=$(${gcmd} app create --creator ${ACCOUNT} --approval-prog <(printf '#pragma version 2\nint 1') --clear-prog <(printf '#pragma version 2\nint 0') --global-byteslices 0 --global-ints 0 --local-byteslices 0 --local-ints 0 | grep Created | awk '{ print $6 }')
+
+# Should succeed to opt in
+${gcmd} app optin --app-id $APPID --from $ACCOUNT
+
+# Succeed in clearing state for the app
+${gcmd} app clear --app-id $APPID --from $ACCOUNT