Chore: Remove an unneeded consensus flag to simplify work on big programs

config/consensus.go

@@ -448,9 +448,9 @@ type ConsensusParams struct {
 	// 6. checking that in the case of going online the VoteFirst is less or equal to the next network round.
 	EnableKeyregCoherencyCheck bool
 
-	// Allow app updates to specify the extra pages they use.  This allows the
-	// update to pass WellFormed(), but they cannot _change_ the extra pages.
-	EnableExtraPagesOnAppUpdate bool
+	// When extra pages were introduced, a bug prevented the extra pages of an
+	// app from being properly removed from the creator upon deletion.
+	EnableProperExtraPageAccounting bool
 
 	// Autoincrements an app's version when the app is updated, careful callers
 	// may avoid making inner calls to apps that have changed.
@@ -1179,8 +1179,8 @@ func initConsensusProtocols() {
 	v29 := v28
 	v29.ApprovedUpgrades = map[protocol.ConsensusVersion]uint64{}
 
-	// Enable ExtraProgramPages for application update
-	v29.EnableExtraPagesOnAppUpdate = true
+	// Fix the accounting bug
+	v29.EnableProperExtraPageAccounting = true
 
 	Consensus[protocol.ConsensusV29] = v29
 

data/transactions/application.go

@@ -271,10 +271,7 @@ func (ac ApplicationCallTxnFields) wellFormed(proto config.ConsensusParams) erro
 			return fmt.Errorf("tx.ExtraProgramPages is immutable")
 		}
 
-		if proto.EnableExtraPagesOnAppUpdate {
-			effectiveEPP = uint32(proto.MaxExtraAppProgramPages)
-		}
-
+		effectiveEPP = uint32(proto.MaxExtraAppProgramPages)
 	}
 
 	// Limit total number of arguments
@@ -320,17 +317,8 @@ func (ac ApplicationCallTxnFields) wellFormed(proto config.ConsensusParams) erro
 		return fmt.Errorf("tx.ExtraProgramPages exceeds MaxExtraAppProgramPages = %d", proto.MaxExtraAppProgramPages)
 	}
 
-	lap := len(ac.ApprovalProgram)
-	lcs := len(ac.ClearStateProgram)
-	pages := int(1 + effectiveEPP)
-	if lap > pages*proto.MaxAppProgramLen {
-		return fmt.Errorf("approval program too long. max len %d bytes", pages*proto.MaxAppProgramLen)
-	}
-	if lcs > pages*proto.MaxAppProgramLen {
-		return fmt.Errorf("clear state program too long. max len %d bytes", pages*proto.MaxAppProgramLen)
-	}
-	if lap+lcs > pages*proto.MaxAppTotalProgramLen {
-		return fmt.Errorf("app programs too long. max total len %d bytes", pages*proto.MaxAppTotalProgramLen)
+	if err := ac.WellSizedPrograms(effectiveEPP, proto); err != nil {
+		return err
 	}
 
 	for i, br := range ac.Boxes {
@@ -354,6 +342,24 @@ func (ac ApplicationCallTxnFields) wellFormed(proto config.ConsensusParams) erro
 	return nil
 }
 
+// WellSizedPrograms checks the sizes of the programs in ac, based on the
+// parameters of proto and returns an error if they are too big.
+func (ac ApplicationCallTxnFields) WellSizedPrograms(extraPages uint32, proto config.ConsensusParams) error {
+	lap := len(ac.ApprovalProgram)
+	lcs := len(ac.ClearStateProgram)
+	pages := int(1 + extraPages)
+	if lap > pages*proto.MaxAppProgramLen {
+		return fmt.Errorf("approval program too long. max len %d bytes", pages*proto.MaxAppProgramLen)
+	}
+	if lcs > pages*proto.MaxAppProgramLen {
+		return fmt.Errorf("clear state program too long. max len %d bytes", pages*proto.MaxAppProgramLen)
+	}
+	if lap+lcs > pages*proto.MaxAppTotalProgramLen {
+		return fmt.Errorf("app programs too long. max total len %d bytes", pages*proto.MaxAppTotalProgramLen)
+	}
+	return nil
+}
+
 // AddressByIndex converts an integer index into an address associated with the
 // transaction. Index 0 corresponds to the transaction sender, and an index > 0
 // corresponds to an offset into txn.Accounts. Returns an error if the index is

data/transactions/application_test.go

@@ -340,7 +340,6 @@ func TestAppCallCreateWellFormed(t *testing.T) {
 func TestWellFormedErrors(t *testing.T) {
 	partitiontest.PartitionTest(t)
 
-	curProto := config.Consensus[protocol.ConsensusCurrentVersion]
 	futureProto := config.Consensus[protocol.ConsensusFuture]
 	protoV27 := config.Consensus[protocol.ConsensusV27]
 	protoV28 := config.Consensus[protocol.ConsensusV28]
@@ -520,21 +519,6 @@ func TestWellFormedErrors(t *testing.T) {
 			proto:         futureProto,
 			expectedError: fmt.Errorf("tx references exceed MaxAppTotalTxnReferences = 8"),
 		},
-		{
-			tx: Transaction{
-				Type:   protocol.ApplicationCallTx,
-				Header: okHeader,
-				ApplicationCallTxnFields: ApplicationCallTxnFields{
-					ApplicationID:     1,
-					ApprovalProgram:   []byte(strings.Repeat("X", 1025)),
-					ClearStateProgram: []byte(strings.Repeat("X", 1025)),
-					ExtraProgramPages: 0,
-					OnCompletion:      UpdateApplicationOC,
-				},
-			},
-			proto:         protoV28,
-			expectedError: fmt.Errorf("app programs too long. max total len %d bytes", curProto.MaxAppProgramLen),
-		},
 		{
 			tx: Transaction{
 				Type:   protocol.ApplicationCallTx,

ledger/apply/application.go

@@ -156,15 +156,9 @@ func deleteApplication(balances Balances, creator basics.Address, appIdx basics.
 	record.TotalAppSchema = totalSchema
 	record.TotalAppParams = basics.SubSaturate(record.TotalAppParams, 1)
 
-	// Delete app's extra program pages
-	totalExtraPages := record.TotalExtraAppPages
-	if totalExtraPages > 0 {
-		proto := balances.ConsensusParams()
-		if proto.EnableExtraPagesOnAppUpdate {
-			extraPages := params.ExtraProgramPages
-			totalExtraPages = basics.SubSaturate(totalExtraPages, extraPages)
-		}
-		record.TotalExtraAppPages = totalExtraPages
+	// There was a short-lived bug so in one version, pages were not deallocated.
+	if balances.ConsensusParams().EnableProperExtraPageAccounting {
+		record.TotalExtraAppPages = basics.SubSaturate(record.TotalExtraAppPages, params.ExtraProgramPages)
 	}
 
 	err = balances.Put(creator, record)
@@ -194,22 +188,13 @@ func updateApplication(ac *transactions.ApplicationCallTxnFields, balances Balan
 		return err
 	}
 
-	// Fill in the new programs
 	proto := balances.ConsensusParams()
-	// when proto.EnableExtraPageOnAppUpdate is false, WellFormed rejects all updates with a multiple-page program
-	if proto.EnableExtraPagesOnAppUpdate {
-		lap := len(ac.ApprovalProgram)
-		lcs := len(ac.ClearStateProgram)
-		pages := int(1 + params.ExtraProgramPages)
-		if lap > pages*proto.MaxAppProgramLen {
-			return fmt.Errorf("updateApplication approval program too long. max len %d bytes", pages*proto.MaxAppProgramLen)
-		}
-		if lcs > pages*proto.MaxAppProgramLen {
-			return fmt.Errorf("updateApplication clear state program too long. max len %d bytes", pages*proto.MaxAppProgramLen)
-		}
-		if lap+lcs > pages*proto.MaxAppTotalProgramLen {
-			return fmt.Errorf("updateApplication app programs too long, %d. max total len %d bytes", lap+lcs, pages*proto.MaxAppTotalProgramLen)
-		}
+
+	// The pre-application well-formedness check rejects big programs
+	// conservatively, it doesn't know the actual params.ExtraProgramPages, so
+	// it allows any programs that fit under the absolute max.
+	if err = ac.WellSizedPrograms(params.ExtraProgramPages, proto); err != nil {
+		return err
 	}
 
 	params.ApprovalProgram = ac.ApprovalProgram

ledger/apply/application_test.go

@@ -719,8 +719,7 @@ func TestAppCallOptIn(t *testing.T) {
 		appIdx++
 		err = optInApplication(b, sender, appIdx, aparams)
 		if cparams.MaxAppsOptedIn > 0 {
-			a.Error(err)
-			a.Contains(err.Error(), "max opted-in apps per acct")
+			a.ErrorContains(err, "max opted-in apps per acct")
 		} else {
 			a.NoError(err)
 		}
@@ -1110,8 +1109,7 @@ func TestAppCallApplyUpdate(t *testing.T) {
 
 		b.pass = true
 		err = ApplicationCall(ac, h, b, ad, 0, ep, txnCounter)
-		a.Error(err)
-		a.Contains(err.Error(), fmt.Sprintf("updateApplication %s program too long", test.name))
+		a.ErrorContains(err, fmt.Sprintf("%s program too long", test.name))
 	}
 
 	b.ResetWrites()
@@ -1137,7 +1135,7 @@ func TestAppCallApplyUpdate(t *testing.T) {
 	}
 	b.pass = true
 	err = ApplicationCall(ac, h, b, ad, 0, ep, txnCounter)
-	a.ErrorContains(err, "updateApplication app programs too long")
+	a.ErrorContains(err, "app programs too long")
 
 }