Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AVM: Implement lsig size pooling #6057

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions cmd/goal/clerk.go
Original file line number Diff line number Diff line change
Expand Up @@ -980,16 +980,6 @@
ops.ReportMultipleErrors(fname, os.Stderr)
reportErrorf("%s: %s", fname, err)
}
_, params := getProto(protoVersion)
if ops.HasStatefulOps {
if len(ops.Program) > config.MaxAvailableAppProgramLen {
reportErrorf(tealAppSize, fname, len(ops.Program), config.MaxAvailableAppProgramLen)
}
} else {
if uint64(len(ops.Program)) > params.LogicSigMaxSize {
reportErrorf(tealLogicSigSize, fname, len(ops.Program), params.LogicSigMaxSize)
}
}

if printWarnings && len(ops.Warnings) != 0 {
for _, warning := range ops.Warnings {
Expand Down Expand Up @@ -1179,14 +1169,19 @@
if timeStamp <= 0 {
timeStamp = time.Now().Unix()
}

lSigPooledSize := 0

Check warning on line 1173 in cmd/goal/clerk.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/clerk.go#L1173

Added line #L1173 was not covered by tests
for i, txn := range stxns {
if txn.Lsig.Blank() {
continue
}
if uint64(txn.Lsig.Len()) > params.LogicSigMaxSize {
lsigLen := txn.Lsig.Len()
lSigPooledSize += lsigLen
if !params.EnableLogicSigSizePooling && uint64(lsigLen) > params.LogicSigMaxSize {

Check warning on line 1180 in cmd/goal/clerk.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/clerk.go#L1178-L1180

Added lines #L1178 - L1180 were not covered by tests
reportErrorf("program size too large: %d > %d", len(txn.Lsig.Logic), params.LogicSigMaxSize)
}
ep := logic.NewSigEvalParams(stxns, &params, logic.NoHeaderLedger{})

err := logic.CheckSignature(i, ep)
if err != nil {
reportErrorf("program failed Check: %s", err)
Expand All @@ -1204,6 +1199,10 @@
fmt.Fprintf(os.Stdout, "ERROR: %s\n", err.Error())
}
}
lSigMaxPooledSize := len(stxns) * int(params.LogicSigMaxSize)
if params.EnableLogicSigSizePooling && lSigPooledSize > lSigMaxPooledSize {
reportErrorf("total lsigs size too large: %d > %d", lSigPooledSize, lSigMaxPooledSize)

Check warning on line 1204 in cmd/goal/clerk.go

View check run for this annotation

Codecov / codecov/patch

cmd/goal/clerk.go#L1202-L1204

Added lines #L1202 - L1204 were not covered by tests
}
giuliop marked this conversation as resolved.
Show resolved Hide resolved

},
}
Expand Down
12 changes: 9 additions & 3 deletions config/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,13 @@ type ConsensusParams struct {
EnableAppCostPooling bool

// EnableLogicSigCostPooling specifies LogicSig budgets are pooled across a
// group. The total available is len(group) * LogicSigMaxCost)
// group. The total available is len(group) * LogicSigMaxCost
EnableLogicSigCostPooling bool

// EnableLogicSigSizePooling specifies LogicSig sizes are pooled across a
// group. The total available is len(group) * LogicSigMaxSize
EnableLogicSigSizePooling bool

// RewardUnit specifies the number of MicroAlgos corresponding to one reward
// unit.
//
Expand Down Expand Up @@ -228,7 +232,7 @@ type ConsensusParams struct {
// 0 for no support, otherwise highest version supported
LogicSigVersion uint64

// len(LogicSig.Logic) + len(LogicSig.Args[*]) must be less than this
// len(LogicSig.Logic) + len(LogicSig.Args[*]) must be less than this (unless pooling is enabled)
LogicSigMaxSize uint64

// sum of estimated op cost must be less than this
Expand Down Expand Up @@ -765,7 +769,7 @@ func checkSetAllocBounds(p ConsensusParams) {
checkSetMax(p.MaxAppProgramLen, &MaxStateDeltaKeys)
checkSetMax(p.MaxAppProgramLen, &MaxEvalDeltaAccounts)
checkSetMax(p.MaxAppProgramLen, &MaxAppProgramLen)
checkSetMax(int(p.LogicSigMaxSize), &MaxLogicSigMaxSize)
checkSetMax((int(p.LogicSigMaxSize) * p.MaxTxGroupSize), &MaxLogicSigMaxSize)
checkSetMax(p.MaxTxnNoteBytes, &MaxTxnNoteBytes)
checkSetMax(p.MaxTxGroupSize, &MaxTxGroupSize)
// MaxBytesKeyValueLen is max of MaxAppKeyLen and MaxAppBytesValueLen
Expand Down Expand Up @@ -1512,6 +1516,8 @@ func initConsensusProtocols() {

vFuture.LogicSigVersion = 11 // When moving this to a release, put a new higher LogicSigVersion here

vFuture.EnableLogicSigSizePooling = true

vFuture.Payouts.Enabled = true
vFuture.Payouts.Percent = 75
vFuture.Payouts.GoOnlineFee = 2_000_000 // 2 algos
Expand Down
12 changes: 10 additions & 2 deletions data/transactions/logic/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,7 @@

var errLogicSigNotSupported = errors.New("LogicSig not supported")
var errTooManyArgs = errors.New("LogicSig has too many arguments")
var errLogicSigArgTooLarge = errors.New("LogicSig argument too large")

// EvalError indicates AVM evaluation failure
type EvalError struct {
Expand Down Expand Up @@ -1305,8 +1306,15 @@
if (cx.EvalParams.Proto == nil) || (cx.EvalParams.Proto.LogicSigVersion == 0) {
return false, errLogicSigNotSupported
}
if cx.txn.Lsig.Args != nil && len(cx.txn.Lsig.Args) > transactions.EvalMaxArgs {
return false, errTooManyArgs
if cx.txn.Lsig.Args != nil {
if len(cx.txn.Lsig.Args) > transactions.EvalMaxArgs {
return false, errTooManyArgs
}
for _, arg := range cx.txn.Lsig.Args {
if len(arg) > transactions.MaxLogicSigArgSize {
giuliop marked this conversation as resolved.
Show resolved Hide resolved
return false, errLogicSigArgTooLarge

Check warning on line 1315 in data/transactions/logic/eval.go

View check run for this annotation

Codecov / codecov/patch

data/transactions/logic/eval.go#L1315

Added line #L1315 was not covered by tests
}
}
}
if verr != nil {
return false, verr
Expand Down
7 changes: 6 additions & 1 deletion data/transactions/logicsig.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ import (
// EvalMaxArgs is the maximum number of arguments to an LSig
const EvalMaxArgs = 255

// MaxLsigArgSize is the maximum size of an argument to an LSig
// We use 4096 to match the maximum size of a TEAL value
// (as defined in `const maxStringSize` in package logic)
const MaxLogicSigArgSize = 4096
jannotti marked this conversation as resolved.
Show resolved Hide resolved

// LogicSig contains logic for validating a transaction.
// LogicSig is signed by an account, allowing delegation of operations.
// OR
Expand All @@ -39,7 +44,7 @@ type LogicSig struct {
Msig crypto.MultisigSig `codec:"msig"`

// Args are not signed, but checked by Logic
Args [][]byte `codec:"arg,allocbound=EvalMaxArgs,allocbound=config.MaxLogicSigMaxSize"`
Args [][]byte `codec:"arg,allocbound=EvalMaxArgs,allocbound=MaxLogicSigArgSize,maxtotalbytes=config.MaxLogicSigMaxSize"`
}

// Blank returns true if there is no content in this LogicSig
Expand Down
10 changes: 5 additions & 5 deletions data/transactions/msgp_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 15 additions & 3 deletions data/transactions/verify/txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
const (
// TxGroupErrorReasonGeneric is a generic (not tracked) reason code
TxGroupErrorReasonGeneric TxGroupErrorReason = iota
// TxGroupErrorReasonNotWellFormed is txn.WellFormed failure
// TxGroupErrorReasonNotWellFormed is txn.WellFormed failure or malformed logic signature
TxGroupErrorReasonNotWellFormed
// TxGroupErrorReasonInvalidFee is invalid fee pooling in transaction group
TxGroupErrorReasonInvalidFee
Expand Down Expand Up @@ -214,6 +214,7 @@

minFeeCount := uint64(0)
feesPaid := uint64(0)
lSigPooledSize := 0
for i, stxn := range stxs {
prepErr := txnBatchPrep(i, groupCtx, verifier)
if prepErr != nil {
Expand All @@ -225,6 +226,17 @@
minFeeCount++
}
feesPaid = basics.AddSaturate(feesPaid, stxn.Txn.Fee.Raw)
lSigPooledSize += stxn.Lsig.Len()
}
if groupCtx.consensusParams.EnableLogicSigSizePooling {
lSigMaxPooledSize := len(stxs) * int(groupCtx.consensusParams.LogicSigMaxSize)
if lSigPooledSize > lSigMaxPooledSize {
errorMsg := fmt.Errorf(
"txgroup had %d bytes of LogicSigs, more than the available pool of %d bytes",
lSigPooledSize, lSigMaxPooledSize,
)
return nil, &TxGroupError{err: errorMsg, GroupIndex: -1, Reason: TxGroupErrorReasonNotWellFormed}

Check warning on line 238 in data/transactions/verify/txn.go

View check run for this annotation

Codecov / codecov/patch

data/transactions/verify/txn.go#L232-L238

Added lines #L232 - L238 were not covered by tests
}
}
feeNeeded, overflow := basics.OMul(groupCtx.consensusParams.MinTxnFee, minFeeCount)
if overflow {
Expand Down Expand Up @@ -361,8 +373,8 @@
if version > groupCtx.consensusParams.LogicSigVersion {
return errors.New("LogicSig.Logic version too new")
}
if uint64(lsig.Len()) > groupCtx.consensusParams.LogicSigMaxSize {
return errors.New("LogicSig.Logic too long")
if !groupCtx.consensusParams.EnableLogicSigSizePooling && uint64(lsig.Len()) > groupCtx.consensusParams.LogicSigMaxSize {
giuliop marked this conversation as resolved.
Show resolved Hide resolved
return errors.New("LogicSig too long")

Check warning on line 377 in data/transactions/verify/txn.go

View check run for this annotation

Codecov / codecov/patch

data/transactions/verify/txn.go#L377

Added line #L377 was not covered by tests
}

err := logic.CheckSignature(gi, groupCtx.evalParams)
Expand Down
8 changes: 4 additions & 4 deletions node/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -824,10 +824,10 @@ func TestMaxSizesCorrect(t *testing.T) {
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)
// the logicsig size is *also* an overestimate, because it thinks that the logicsig and
// the logicsig args can both be up to to MaxLogicSigMaxSize, but that's the max for
// them combined, so it double counts and we have to subtract one.
maxCombinedTxnSize -= uint64(config.MaxLogicSigMaxSize)
giuliop marked this conversation as resolved.
Show resolved Hide resolved

// 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
Expand Down
19 changes: 0 additions & 19 deletions test/e2e-go/cli/goal/expect/tealConsensusTest.exp
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,6 @@ if { [catch {
"\n" { ::AlgorandGoal::Abort $expect_out(buffer) }
}

teal "$TEST_ROOT_DIR/big-sig.teal" 2 1001 1
spawn goal clerk compile "$TEST_ROOT_DIR/big-sig.teal"
expect {
-re {[A-Z2-9]{58}} { ::AlgorandGoal::Abort "hash" }
-re {.*logicsig program size too large} { puts "bigsigcheck: pass" }
eof { ::AlgorandGoal::Abort $expect_out(buffer) }
"\n" { ::AlgorandGoal::Abort $expect_out(buffer) }
}

teal "$TEST_ROOT_DIR/barely-fits-app.teal" 2 4090 1 "int 0\nbalance\npop\n"
spawn goal clerk compile "$TEST_ROOT_DIR/barely-fits-app.teal"
expect {
Expand All @@ -71,16 +62,6 @@ if { [catch {
"\n" { ::AlgorandGoal::Abort $expect_out(buffer) }
}

# MaxAppProgramLen = 2K * (1 + 3 pages max)
teal "$TEST_ROOT_DIR/big-app.teal" 2 8192 1 "int 0\nbalance\npop\n"
spawn goal clerk compile "$TEST_ROOT_DIR/big-app.teal"
expect {
-re {[A-Z2-9]{58}} { ::AlgorandGoal::Abort "hash" }
-re {.*app program size too large} { puts "bigappcheck: pass" }
eof { ::AlgorandGoal::Abort $expect_out(buffer) }
"\n" { ::AlgorandGoal::Abort $expect_out(buffer) }
}

# Test cost limits during dryrun
exec goal clerk send -F "$TEST_ROOT_DIR/small-sig.teal" -t GXBNLU4AXQABPLHXJDMTG2YXSDT4EWUZACT7KTPFXDQW52XPTIUS5OZ5HQ -a 100 -d $TEST_PRIMARY_NODE_DIR -o $TEST_ROOT_DIR/small-sig.tx
spawn goal clerk dryrun -t $TEST_ROOT_DIR/small-sig.tx
Expand Down
Loading