ExtraProgramPages not usable after Creation

[jeapostrophe]

You can set ExtraProgramPages to something greater than 0 at creation time, but you can't update an application and use extra space you allocated

If you look at this code:

go-algorand/data/transactions/transaction.go

Lines 353 to 408 in c7b017a

	if tx.ExtraProgramPages != 0 {
	return fmt.Errorf("tx.ExtraProgramPages is immutable")
	}
	}
	// Limit total number of arguments
	if len(tx.ApplicationArgs) > proto.MaxAppArgs {
	return fmt.Errorf("too many application args, max %d", proto.MaxAppArgs)
	}
	// Sum up argument lengths
	var argSum uint64
	for _, arg := range tx.ApplicationArgs {
	argSum = basics.AddSaturate(argSum, uint64(len(arg)))
	}
	// Limit total length of all arguments
	if argSum > uint64(proto.MaxAppTotalArgLen) {
	return fmt.Errorf("application args total length too long, max len %d bytes", proto.MaxAppTotalArgLen)
	}
	// Limit number of accounts referred to in a single ApplicationCall
	if len(tx.Accounts) > proto.MaxAppTxnAccounts {
	return fmt.Errorf("tx.Accounts too long, max number of accounts is %d", proto.MaxAppTxnAccounts)
	}
	// Limit number of other app global states referred to
	if len(tx.ForeignApps) > proto.MaxAppTxnForeignApps {
	return fmt.Errorf("tx.ForeignApps too long, max number of foreign apps is %d", proto.MaxAppTxnForeignApps)
	}
	if len(tx.ForeignAssets) > proto.MaxAppTxnForeignAssets {
	return fmt.Errorf("tx.ForeignAssets too long, max number of foreign assets is %d", proto.MaxAppTxnForeignAssets)
	}
	// Limit the sum of all types of references that bring in account records
	if len(tx.Accounts)+len(tx.ForeignApps)+len(tx.ForeignAssets) > proto.MaxAppTotalTxnReferences {
	return fmt.Errorf("tx has too many references, max is %d", proto.MaxAppTotalTxnReferences)
	}
	if tx.ExtraProgramPages > uint32(proto.MaxExtraAppProgramPages) {
	return fmt.Errorf("tx.ExtraProgramPages too large, max number of extra pages is %d", proto.MaxExtraAppProgramPages)
	}
	lap := len(tx.ApprovalProgram)
	lcs := len(tx.ClearStateProgram)
	pages := int(1 + tx.ExtraProgramPages)
	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)
	}

You can see that

• L348 - is true when you are updating
• L353 - guarantees that tx.ExtraProgramPages is 0
• L399 - is 0 because of above
• L406 - is going to be true (i.e. error) because of that

I believe that this is not a problem when you are not updating (i.e. when you are calling) because tx.ApprovalProgram and tx.ClearProgram are expected to be 0 since they aren't being updated.

I believe that the fix is to look up the actual application ExtraProgramPages when you aren't updating and use the stored value

[jeapostrophe]

I'll work on a patch and get back to you about my proposed fixed, but I figured learning about the problem fast is good :)

[jeapostrophe]

@shiqizng

[jannotti]

I believe that the fix is to look up the actual application ExtraProgramPages when you aren't updating and use the stored value

That sounds right to me.

[jeapostrophe]

I spent some time working on a patch for this and I now think that WellFormed should not be checking the program lengths on update like this. Basically WellFormed says // WellFormed checks that the transaction looks reasonable on its own (but not necessarily valid against the actual ledger). It does not check signatures. The only way to do this check is to look at the current ledger, which is not available to this function. So instead, it should make sure the program is not more than is possible and then updateApplication in application.go should check to make sure the new programs are valid

[jeapostrophe]

This is the patch I'm using and it is working well --- https://gist.github.com/94149c776903646a9d8e0120811b5e02

[shiqizng]

the fix looks good to me.