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

Merged
merged 22 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ec90b93
Implement lsig size pooling
giuliop Jul 2, 2024
a2e7f4d
Merge remote-tracking branch 'upstream/master' into pool-max-logicSig…
giuliop Jul 10, 2024
129bcab
Merge remote-tracking branch 'upstream/master' into pool-max-logicSig…
giuliop Jul 21, 2024
bc74e17
Implement requested changes
giuliop Jul 21, 2024
72ffe79
Merge remote-tracking branch 'upstream/master' into pool-max-logicSig…
giuliop Jul 29, 2024
e6bcb26
Implement new max size for LogicSigMaxSize
giuliop Jul 29, 2024
8fa1b7f
Merge remote-tracking branch 'upstream/master' into pool-max-logicSig…
giuliop Aug 28, 2024
16793dc
Make goal more permissive when compiling logicsigs
giuliop Aug 28, 2024
16ef899
Implement max size for logicsig args of 4096 bytes
giuliop Aug 28, 2024
91d6e09
Make TxnTagMaxSize same as max block size
giuliop Aug 28, 2024
5a1eeaf
Revert "Make TxnTagMaxSize same as max block size"
giuliop Sep 9, 2024
11baff1
Merge remote-tracking branch 'upstream/master' into pool-max-logicSig…
giuliop Sep 9, 2024
39600e5
Update TestMaxSizesCorrect with logicsig pooling
giuliop Sep 9, 2024
4de5dd0
Don't test app/lsig max size in goal clerk compile
giuliop Sep 10, 2024
7953862
Add integration test for lsig size pooling
giuliop Sep 14, 2024
2265d17
Refactor
giuliop Sep 25, 2024
6206c38
Fix linter warnings
giuliop Oct 23, 2024
c3c2224
Make GenerateProgramOfSize accessible to other packages
giuliop Oct 23, 2024
1872212
Add unit test for lsig size
giuliop Oct 23, 2024
34d6079
Add unit test for lsig arg size
giuliop Oct 23, 2024
f0b4a1e
Fix typo in comments
giuliop Oct 23, 2024
0b5564e
Merge remote-tracking branch 'upstream/master' into pool-max-logicSig…
giuliop Oct 23, 2024
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
giuliop marked this conversation as resolved.
Show resolved Hide resolved
}
}
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