performance: do not report extra details for failed app txns (#6171)

Co-authored-by: Gary Malouf <982483+gmalouf@users.noreply.github.com>
Co-authored-by: cce <51567+cce@users.noreply.github.com>

config/localTemplate.go

@@ -44,7 +44,7 @@ type Local struct {
 	// Version tracks the current version of the defaults so we can migrate old -> new
 	// This is specifically important whenever we decide to change the default value
 	// for an existing parameter. This field tag must be updated any time we add a new version.
-	Version uint32 `version[0]:"0" version[1]:"1" version[2]:"2" version[3]:"3" version[4]:"4" version[5]:"5" version[6]:"6" version[7]:"7" version[8]:"8" version[9]:"9" version[10]:"10" version[11]:"11" version[12]:"12" version[13]:"13" version[14]:"14" version[15]:"15" version[16]:"16" version[17]:"17" version[18]:"18" version[19]:"19" version[20]:"20" version[21]:"21" version[22]:"22" version[23]:"23" version[24]:"24" version[25]:"25" version[26]:"26" version[27]:"27" version[28]:"28" version[29]:"29" version[30]:"30" version[31]:"31" version[32]:"32" version[33]:"33" version[34]:"34"`
+	Version uint32 `version[0]:"0" version[1]:"1" version[2]:"2" version[3]:"3" version[4]:"4" version[5]:"5" version[6]:"6" version[7]:"7" version[8]:"8" version[9]:"9" version[10]:"10" version[11]:"11" version[12]:"12" version[13]:"13" version[14]:"14" version[15]:"15" version[16]:"16" version[17]:"17" version[18]:"18" version[19]:"19" version[20]:"20" version[21]:"21" version[22]:"22" version[23]:"23" version[24]:"24" version[25]:"25" version[26]:"26" version[27]:"27" version[28]:"28" version[29]:"29" version[30]:"30" version[31]:"31" version[32]:"32" version[33]:"33" version[34]:"34" version[35]:"35"`
 
 	// Archival nodes retain a full copy of the block history. Non-Archival nodes will delete old blocks and only retain what's need to properly validate blockchain messages (the precise number of recent blocks depends on the consensus parameters. Currently the last 1321 blocks are required). This means that non-Archival nodes require significantly less storage than Archival nodes.  If setting this to true for the first time, the existing ledger may need to be deleted to get the historical values stored as the setting only affects current blocks forward. To do this, shutdown the node and delete all .sqlite files within the data/testnet-version directory, except the crash.sqlite file. Restart the node and wait for the node to sync.
 	Archival bool `version[0]:"false"`
@@ -66,7 +66,7 @@ type Local struct {
 	PublicAddress string `version[0]:""`
 
 	// MaxConnectionsPerIP is the maximum number of connections allowed per IP address.
-	MaxConnectionsPerIP int `version[3]:"30" version[27]:"15"`
+	MaxConnectionsPerIP int `version[3]:"30" version[27]:"15" version[35]:"8"`
 
 	// PeerPingPeriodSeconds is deprecated and unused.
 	PeerPingPeriodSeconds int `version[0]:"0"`
@@ -171,7 +171,7 @@ type Local struct {
 	EndpointAddress string `version[0]:"127.0.0.1:0"`
 
 	// Respond to Private Network Access preflight requests sent to the node. Useful when a public website is trying to access a node that's hosted on a local network.
-	EnablePrivateNetworkAccessHeader bool `version[34]:"false"`
+	EnablePrivateNetworkAccessHeader bool `version[35]:"false"`
 
 	// RestReadTimeoutSeconds is passed to the API servers rest http.Server implementation.
 	RestReadTimeoutSeconds int `version[4]:"15"`
@@ -255,6 +255,12 @@ type Local struct {
 	// EnableTxBacklogAppRateLimiting controls if an app rate limiter should be attached to the tx backlog enqueue process
 	EnableTxBacklogAppRateLimiting bool `version[32]:"true"`
 
+	// TxBacklogAppRateLimitingCountERLDrops feeds messages dropped by the ERL congestion manager & rate limiter (enabled by
+	// EnableTxBacklogRateLimiting) to the app rate limiter (enabled by EnableTxBacklogAppRateLimiting), so that all TX messages
+	// are counted. This provides more accurate rate limiting for the app rate limiter, at the potential expense of additional
+	// deserialization overhead.
+	TxBacklogAppRateLimitingCountERLDrops bool `version[35]:"false"`
+
 	// EnableTxBacklogRateLimiting controls if a rate limiter and congestion manager should be attached to the tx backlog enqueue process
 	// if enabled, the over-all TXBacklog Size will be larger by MAX_PEERS*TxBacklogReservedCapacityPerPeer
 	EnableTxBacklogRateLimiting bool `version[27]:"false" version[30]:"true"`

config/local_defaults.go

@@ -20,7 +20,7 @@
 package config
 
 var defaultLocal = Local{
-	Version:                                    34,
+	Version:                                    35,
 	AccountUpdatesStatsInterval:                5000000000,
 	AccountsRebuildSynchronousMode:             1,
 	AgreementIncomingBundlesQueueLength:        15,
@@ -111,7 +111,7 @@ var defaultLocal = Local{
 	MaxAcctLookback:                            4,
 	MaxBlockHistoryLookback:                    0,
 	MaxCatchpointDownloadDuration:              43200000000000,
-	MaxConnectionsPerIP:                        15,
+	MaxConnectionsPerIP:                        8,
 	MinCatchpointFileDownloadBytesPerSecond:    20480,
 	NetAddress:                                 "",
 	NetworkMessageTraceServer:                  "",
@@ -148,6 +148,7 @@ var defaultLocal = Local{
 	TrackerDBDir:                               "",
 	TransactionSyncDataExchangeRate:            0,
 	TransactionSyncSignificantMessageThreshold: 0,
+	TxBacklogAppRateLimitingCountERLDrops:      false,
 	TxBacklogAppTxPerSecondRate:                100,
 	TxBacklogAppTxRateLimiterMaxSize:           1048576,
 	TxBacklogRateLimitingCongestionPct:         50,

data/pools/transactionPool.go

@@ -62,6 +62,7 @@ type TransactionPool struct {
 	cond                   sync.Cond
 	expiredTxCount         map[basics.Round]int
 	pendingBlockEvaluator  BlockEvaluator
+	evalTracer             logic.EvalTracer
 	numPendingWholeBlocks  basics.Round
 	feePerByte             atomic.Uint64
 	feeThresholdMultiplier uint64
@@ -140,6 +141,9 @@ func MakeTransactionPool(ledger *ledger.Ledger, cfg config.Local, log logging.Lo
 		log:                  log,
 		vac:                  vac,
 	}
+	if cfg.EnableDeveloperAPI {
+		pool.evalTracer = logic.EvalErrorDetailsTracer{}
+	}
 	pool.cond.L = &pool.mu
 	pool.assemblyCond.L = &pool.assemblyMu
 	pool.recomputeBlockEvaluator(nil, 0)
@@ -732,7 +736,7 @@ func (pool *TransactionPool) recomputeBlockEvaluator(committedTxIDs map[transact
 	if hint < 0 || int(knownCommitted) < 0 {
 		hint = 0
 	}
-	pool.pendingBlockEvaluator, err = pool.ledger.StartEvaluator(next.BlockHeader, hint, 0, nil)
+	pool.pendingBlockEvaluator, err = pool.ledger.StartEvaluator(next.BlockHeader, hint, 0, pool.evalTracer)
 	if err != nil {
 		// The pendingBlockEvaluator is an interface, and in case of an evaluator error
 		// we want to remove the interface itself rather then keeping an interface

data/transactions/logic/eval.go

@@ -1028,8 +1028,16 @@ func (err EvalError) Unwrap() error {
 }
 
 func (cx *EvalContext) evalError(err error) error {
-	pc, det := cx.pcDetails()
-	details := fmt.Sprintf("pc=%d, opcodes=%s", pc, det)
+	var pc int
+	var details string
+	if cx.Tracer != nil && cx.Tracer.DetailedEvalErrors() {
+		var det string
+		pc, det = cx.pcDetails()
+		details = fmt.Sprintf("pc=%d, opcodes=%s", pc, det)
+	} else {
+		pc = cx.pc
+		details = fmt.Sprintf("pc=%d", pc)
+	}
 
 	err = basics.Annotate(err,
 		"pc", pc,

data/transactions/logic/evalStateful_test.go

@@ -442,6 +442,7 @@ func testApps(t *testing.T, programs []string, txgroup []transactions.SignedTxn,
 		}
 	}
 	ep := NewAppEvalParams(transactions.WrapSignedTxnsWithAD(txgroup), proto, &transactions.SpecialAddresses{})
+	ep.Tracer = EvalErrorDetailsTracer{}
 	if ledger == nil {
 		ledger = NewLedger(nil)
 	}

data/transactions/logic/mocktracer/tracer.go

@@ -232,6 +232,9 @@ func (d *Tracer) AfterBlock(hdr *bookkeeping.BlockHeader) {
 	d.Events = append(d.Events, AfterBlock(hdr.Round))
 }
 
+// DetailedEvalErrors returns true, enabling detailed errors in tests.
+func (d *Tracer) DetailedEvalErrors() bool { return true }
+
 // copyDeltas makes a deep copy of the given ledgercore.StateDelta pointer, if it's not nil.
 // This is inefficient, but it should only be used for testing.
 func copyDeltas(deltas *ledgercore.StateDelta) *ledgercore.StateDelta {

data/transactions/logic/tracer.go

@@ -162,6 +162,10 @@ type EvalTracer interface {
 	// AfterBlock is called after the block has finished evaluation. It will not be called in the event that an evalError
 	// stops evaluation of the block.
 	AfterBlock(hdr *bookkeeping.BlockHeader)
+
+	// DetailedEvalErrors permits the tracer to enable detailed EvalError messages (including PC with disassembled
+	// opcodes) by returning true.
+	DetailedEvalErrors() bool
 }
 
 // NullEvalTracer implements EvalTracer, but all of its hook methods do nothing
@@ -198,3 +202,12 @@ func (n NullEvalTracer) AfterOpcode(cx *EvalContext, evalError error) {}
 
 // AfterBlock does nothing
 func (n NullEvalTracer) AfterBlock(hdr *bookkeeping.BlockHeader) {}
+
+// DetailedEvalErrors does nothing
+func (n NullEvalTracer) DetailedEvalErrors() bool { return false }
+
+// EvalErrorDetailsTracer enables disassembled details in EvalError messages, and nothing else.
+type EvalErrorDetailsTracer struct{ NullEvalTracer }
+
+// DetailedEvalErrors returns true.
+func (EvalErrorDetailsTracer) DetailedEvalErrors() bool { return true }

data/txHandler.go

@@ -132,6 +132,7 @@ type TxHandler struct {
 	erl                        *util.ElasticRateLimiter
 	appLimiter                 *appRateLimiter
 	appLimiterBacklogThreshold int
+	appLimiterCountERLDrops    bool
 
 	// batchVerifier provides synchronous verification of transaction groups, used only by pubsub validation in validateIncomingTxMessage.
 	batchVerifier verify.TxnGroupBatchSigVerifier
@@ -209,6 +210,7 @@ func MakeTxHandler(opts TxHandlerOpts) (*TxHandler, error) {
 			)
 			// set appLimiter triggering threshold at 50% of the base backlog size
 			handler.appLimiterBacklogThreshold = int(float64(opts.Config.TxBacklogSize) * float64(opts.Config.TxBacklogRateLimitingCongestionPct) / 100)
+			handler.appLimiterCountERLDrops = opts.Config.TxBacklogAppRateLimitingCountERLDrops
 		}
 	}
 
@@ -622,15 +624,17 @@ func (handler *TxHandler) incomingMsgDupCheck(data []byte) (*crypto.Digest, bool
 // - a boolean indicating if the sender is rate limited
 func (handler *TxHandler) incomingMsgErlCheck(sender network.DisconnectablePeer) (*util.ErlCapacityGuard, bool) {
 	var capguard *util.ErlCapacityGuard
+	var isCMEnabled bool
 	var err error
 	if handler.erl != nil {
 		congestedERL := float64(cap(handler.backlogQueue))*handler.backlogCongestionThreshold < float64(len(handler.backlogQueue))
 		// consume a capacity unit
 		// if the elastic rate limiter cannot vend a capacity, the error it returns
 		// is sufficient to indicate that we should enable Congestion Control, because
 		// an issue in vending capacity indicates the underlying resource (TXBacklog) is full
-		capguard, err = handler.erl.ConsumeCapacity(sender.(util.ErlClient))
-		if err != nil {
+		capguard, isCMEnabled, err = handler.erl.ConsumeCapacity(sender.(util.ErlClient))
+		if err != nil || // did ERL ask to enable congestion control?
+			(!isCMEnabled && congestedERL) { // is CM not currently enabled, but queue is congested?
 			handler.erl.EnableCongestionControl()
 			// if there is no capacity, it is the same as if we failed to put the item onto the backlog, so report such
 			transactionMessagesDroppedFromBacklog.Inc(nil)
@@ -741,6 +745,12 @@ func (handler *TxHandler) processIncomingTxn(rawmsg network.IncomingMessage) net
 	}()
 
 	if shouldDrop {
+		if handler.appLimiterCountERLDrops {
+			// decode and let ARL count this txgroup, even though ERL is dropping it
+			if unverifiedTxGroup, _, invalid := decodeMsg(rawmsg.Data); !invalid {
+				handler.incomingTxGroupAppRateLimit(unverifiedTxGroup, rawmsg.Sender)
+			}
+		}
 		// this TX message was rate-limited by ERL
 		return network.OutgoingMessage{Action: network.Ignore}
 	}

installer/config.json.example

@@ -1,5 +1,5 @@
 {
-    "Version": 34,
+    "Version": 35,
     "AccountUpdatesStatsInterval": 5000000000,
     "AccountsRebuildSynchronousMode": 1,
     "AgreementIncomingBundlesQueueLength": 15,
@@ -90,7 +90,7 @@
     "MaxAcctLookback": 4,
     "MaxBlockHistoryLookback": 0,
     "MaxCatchpointDownloadDuration": 43200000000000,
-    "MaxConnectionsPerIP": 15,
+    "MaxConnectionsPerIP": 8,
     "MinCatchpointFileDownloadBytesPerSecond": 20480,
     "NetAddress": "",
     "NetworkMessageTraceServer": "",
@@ -127,6 +127,7 @@
     "TrackerDBDir": "",
     "TransactionSyncDataExchangeRate": 0,
     "TransactionSyncSignificantMessageThreshold": 0,
+    "TxBacklogAppRateLimitingCountERLDrops": false,
     "TxBacklogAppTxPerSecondRate": 100,
     "TxBacklogAppTxRateLimiterMaxSize": 1048576,
     "TxBacklogRateLimitingCongestionPct": 50,

ledger/simple_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/algorand/go-algorand/data/bookkeeping"
 	"github.com/algorand/go-algorand/data/committee"
 	"github.com/algorand/go-algorand/data/transactions"
+	"github.com/algorand/go-algorand/data/transactions/logic"
 	"github.com/algorand/go-algorand/data/transactions/verify"
 	"github.com/algorand/go-algorand/data/txntest"
 	"github.com/algorand/go-algorand/ledger/eval"
@@ -91,6 +92,7 @@ func nextBlock(t testing.TB, ledger *Ledger) *eval.BlockEvaluator {
 	eval, err := eval.StartEvaluator(ledger, nextHdr, eval.EvaluatorOptions{
 		Generate: true,
 		Validate: true, // Do the complete checks that a new txn would be subject to
+		Tracer:   logic.EvalErrorDetailsTracer{},
 	})
 	require.NoError(t, err)
 	return eval

ledger/simulation/tracer.go

@@ -555,3 +555,5 @@ func (tracer *evalTracer) AfterProgram(cx *logic.EvalContext, pass bool, evalErr
 		}
 	}
 }
+
+func (tracer *evalTracer) DetailedEvalErrors() bool { return true }

test/testdata/configs/config-v34.json

@@ -57,7 +57,6 @@
     "EnableP2P": false,
     "EnableP2PHybridMode": false,
     "EnablePingHandler": true,
-    "EnablePrivateNetworkAccessHeader": false,
     "EnableProcessBlockStats": false,
     "EnableProfiler": false,
     "EnableRequestLogger": false,