AVM: Derive looser, but more principled, checks of txn max size

node/node_test.go

@@ -33,9 +33,11 @@ import (
 	"github.com/algorand/go-algorand/agreement"
 	"github.com/algorand/go-algorand/config"
 	"github.com/algorand/go-algorand/crypto"
+	csp "github.com/algorand/go-algorand/crypto/stateproof"
 	"github.com/algorand/go-algorand/data/account"
 	"github.com/algorand/go-algorand/data/basics"
 	"github.com/algorand/go-algorand/data/bookkeeping"
+	"github.com/algorand/go-algorand/data/stateproofmsg"
 	"github.com/algorand/go-algorand/data/transactions"
 	"github.com/algorand/go-algorand/logging"
 	"github.com/algorand/go-algorand/network"
@@ -805,11 +807,48 @@ func TestMaxSizesCorrect(t *testing.T) {
 	require.Equal(t, ppSize, protocol.ProposalPayloadTag.MaxMessageSize())
 	spSize := uint64(stateproof.SigFromAddrMaxSize())
 	require.Equal(t, spSize, protocol.StateProofSigTag.MaxMessageSize())
-	txSize := uint64(transactions.SignedTxnMaxSize())
-	require.Equal(t, txSize, protocol.TxnTag.MaxMessageSize())
 	msSize := uint64(crypto.DigestMaxSize())
 	require.Equal(t, msSize, protocol.MsgDigestSkipTag.MaxMessageSize())
 
+	// We want to check that the TxnTag's max size is big enough, but it is
+	// foolish to try to be exact here.  We will confirm that it is bigger that
+	// a stateproof txn (the biggest kind, which can only appear by itself), and
+	// that it is bigger than 16 times the largest transaction other than
+	// stateproof txn.
+	txTagMax := protocol.TxnTag.MaxMessageSize()
+
+	// SignedTxnMaxSize() is an overestimate of a single transaction because it
+	// includes fields from all the different types of signatures, and types of
+	// transactions. First, we remove the aspects of the overestimate that come
+	// from the multiple signature types.
+	maxCombinedTxnSize := uint64(transactions.SignedTxnMaxSize())
+	// subtract out the two smaller signature sizes (logicsig is biggest, it can *contain* the others)
+	maxCombinedTxnSize -= uint64(crypto.SignatureMaxSize() + crypto.MultisigSigMaxSize())
+	// the logicsig size is *also* an overestimate, because it thinks each
+	// logicsig arg can be big, but really the sum of the args and the program
+	// has a max size.
+	maxCombinedTxnSize -= uint64(transactions.EvalMaxArgs * config.MaxLogicSigMaxSize)
+
+	// maxCombinedTxnSize is still an overestimate because it assumes all txn
+	// type fields can be in the same txn.  That's not true, but it provides an
+	// upper bound on the size of ONE transaction, even if the txn is a
+	// stateproof, which is big.  Ensure our constant is big enough to hold one.
+	require.Greater(t, txTagMax, maxCombinedTxnSize)
+
+	// we actually have to hold 16 txns, but in the case of multiple txns in a
+	// group, none can be stateproofs. So derive maxMinusSP, which is a per txn
+	// size estimate that excludes stateproof fields.
+	spTxnSize := uint64(csp.StateProofMaxSize() + stateproofmsg.MessageMaxSize())
+	maxMinusSP := maxCombinedTxnSize - spTxnSize
+	require.Greater(t, txTagMax, 16*maxMinusSP)
+	// when we do logisig pooling, 16*maxMinusSP may be a large overshoot, since
+	// it will assume we can have a big logicsig in _each_ of the 16.  It
+	// probably won't matter, since stateproof will still swamp it.  But if so,
+	// remove 15 * MaxLogicSigMaxSize.
+
+	// but we're not crazy. whichever of those is bigger - we don't need to be twice as big as that
+	require.Less(t, txTagMax, 2*max(maxCombinedTxnSize, 16*maxMinusSP))
+
 	// UE is a handrolled message not using msgp
 	// including here for completeness ensured by protocol.TestMaxSizesTested
 	ueSize := uint64(67)

protocol/tags.go

@@ -84,10 +84,23 @@ const StateProofSigTagMaxSize = 6378
 // Matches  current network.MaxMessageLength
 const TopicMsgRespTagMaxSize = 6 * 1024 * 1024
 
-// TxnTagMaxSize is the maximum size of a TxnTag message. This is equal to SignedTxnMaxSize()
-// which is size of just a single message containing maximum Stateproof. Since Stateproof
-// transactions can't be batched we don't need to multiply by MaxTxnBatchSize.
-const TxnTagMaxSize = 4620031
+// TxnTagMaxSize is the maximum size of a TxnTag message. The TxnTag is used to
+// send entire transaction groups. So, naively, we might set it to the maximum
+// group size times the maximum transaction size (plus a little bit for msgpack
+// encoding).  But there are several reasons not to do that.  First, the
+// function we have for estimating max transaction size
+// (transactions.SignedTxnMaxSize())) wildly overestimates the maximum
+// transaction size because it is generated code that assumes _every_
+// transaction field can be set, but each transaction type has mutually
+// exclusive fields. Second, the stateproof transaction is the biggest
+// transaction by far, but it can only appear as a singleton, so it would not
+// make sense to multiply it by 16.  Finally, we're going to pool logicsig code
+// size, so while it's true that one transaction in a group could have a 16k
+// logicsig, that would only be true if the other transactions had 0 bytes of
+// logicsig.  So we will use a bound that is a bit bigger that a txn group can
+// be, but avoid trying to be precise.  See TestMaxSizesCorrect for the detailed
+// reasoning.
+const TxnTagMaxSize = 5_000_000
 
 // UniEnsBlockReqTagMaxSize is the maximum size of a UniEnsBlockReqTag message
 const UniEnsBlockReqTagMaxSize = 67

protocol/tags_test.go

@@ -169,6 +169,10 @@ func TestMaxSizesTested(t *testing.T) {
 	}
 
 	for _, tag := range constTags {
+		if tag == "TxnTag" {
+			// TxnTag is tested in a looser way in TestMaxSizesCorrect
+			continue
+		}
 		require.Truef(t, tagsFound[tag], "Tag %s does not have a corresponding test in TestMaxSizesCorrect", tag)
 	}
 }