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

Inner groups #3009

Merged
merged 10 commits into from
Oct 7, 2021
Merged
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
5 changes: 3 additions & 2 deletions data/transactions/logic/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -466,9 +466,10 @@ transaction types, are rejected by `itxn_submit`.

| Op | Description |
| --- | --- |
| `itxn_begin` | begin preparation of a new inner transaction |
| `itxn_begin` | begin preparation of a new inner transaction in a new transaction group |
| `itxn_next` | begin preparation of a new inner transaction in the same transaction group |
| `itxn_field f` | set field F of the current inner transaction to X |
| `itxn_submit` | execute the current inner transaction. Fail if 16 inner transactions have already been executed, or if the transaction itself fails. |
| `itxn_submit` | execute the current inner transaction group. Fail if executing this group would exceed 16 total inner transactions, or if any transaction in the group fails. |
| `itxn f` | push field F of the last inner transaction to stack |
| `itxna f i` | push Ith value of the array field F of the last inner transaction to stack |

Expand Down
13 changes: 11 additions & 2 deletions data/transactions/logic/TEAL_opcodes.md
Original file line number Diff line number Diff line change
Expand Up @@ -1296,7 +1296,7 @@ bitlen interprets arrays as big-endian integers, unlike setbit/getbit
- Opcode: 0xb1
- Pops: _None_
- Pushes: _None_
- begin preparation of a new inner transaction
- begin preparation of a new inner transaction in a new transaction group
- LogicSigVersion >= 5
- Mode: Application

Expand All @@ -1318,7 +1318,7 @@ bitlen interprets arrays as big-endian integers, unlike setbit/getbit
- Opcode: 0xb3
- Pops: _None_
- Pushes: _None_
- execute the current inner transaction. Fail if 16 inner transactions have already been executed, or if the transaction itself fails.
- execute the current inner transaction group. Fail if executing this group would exceed 16 total inner transactions, or if any transaction in the group fails.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you consider changing the name of "itxn_submit", since "itxn" in the other opcodes is referring to a single inner transaction, whereas here it's referring to the whole group of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I originally thought I'd be explicit about the whole thing, wrap a bunch of matched itxn_begin and itxn_end pairs with itxg_begin, itxg_submit. But I couldn't justify all those opcodes once I started trying to write apps with them.

This seemed like the minimal change possible, and you can use itxn_submit to submit a single transaction, so I didn't want to waste an opcode on the distinction. But it's not crazy if people think we should go that way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally like it the way you did it, I think the additional opcodes aren't worth the minimal increase in explicitness/clarity.

- LogicSigVersion >= 5
- Mode: Application

Expand All @@ -1342,6 +1342,15 @@ bitlen interprets arrays as big-endian integers, unlike setbit/getbit
- LogicSigVersion >= 5
- Mode: Application

## itxn_next

- Opcode: 0xb6
- Pops: _None_
- Pushes: _None_
- begin preparation of a new inner transaction in the same transaction group
- LogicSigVersion >= 6
- Mode: Application

## txnas f

- Opcode: 0xc0 {uint8 transaction field index}
Expand Down
3 changes: 2 additions & 1 deletion data/transactions/logic/assembler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ itxna Logs 3
`

const v6Nonsense = v5Nonsense + `
itxn_next
`

var nonsense = map[uint64]string{
Expand All @@ -360,7 +361,7 @@ var compiled = map[uint64]string{
3: "032008b7a60cf8acd19181cf959a12f8acd19181cf951af8acd19181cf15f8acd191810f01020026050212340c68656c6c6f20776f726c6421208dae2087fbba51304eb02b91f656948397a7946390e8cb70fc9ea4d95f92251d024242047465737400320032013202320328292929292a0431003101310231043105310731083109310a310b310c310d310e310f3111311231133114311533000033000133000233000433000533000733000833000933000a33000b33000c33000d33000e33000f3300113300123300133300143300152d2e0102222324252104082209240a220b230c240d250e230f23102311231223132314181b1c2b171615400003290349483403350222231d4a484848482a50512a63222352410003420000432105602105612105270463484821052b62482b642b65484821052b2106662b21056721072b682b692107210570004848210771004848361c0037001a0031183119311b311d311e311f3120210721051e312131223123312431253126312731283129312a312b312c312d312e312f4478222105531421055427042106552105082106564c4d4b02210538212106391c0081e80780046a6f686e",
4: "042004010200b7a60c26040242420c68656c6c6f20776f726c6421208dae2087fbba51304eb02b91f656948397a7946390e8cb70fc9ea4d95f92251d047465737400320032013202320380021234292929292a0431003101310231043105310731083109310a310b310c310d310e310f3111311231133114311533000033000133000233000433000533000733000833000933000a33000b33000c33000d33000e33000f3300113300123300133300143300152d2e01022581f8acd19181cf959a1281f8acd19181cf951a81f8acd19181cf1581f8acd191810f082209240a220b230c240d250e230f23102311231223132314181b1c28171615400003290349483403350222231d4a484848482a50512a632223524100034200004322602261222b634848222862482864286548482228236628226724286828692422700048482471004848361c0037001a0031183119311b311d311e311f312024221e312131223123312431253126312731283129312a312b312c312d312e312f44782522531422542b2355220823564c4d4b0222382123391c0081e80780046a6f686e2281d00f24231f880003420001892223902291922394239593a0a1a2a3a4a5a6a7a8a9aaabacadae23af3a00003b003c003d8164",
5: "052004010002b7a60c26050242420c68656c6c6f20776f726c6421070123456789abcd208dae2087fbba51304eb02b91f656948397a7946390e8cb70fc9ea4d95f92251d047465737400320032013202320380021234292929292b0431003101310231043105310731083109310a310b310c310d310e310f3111311231133114311533000033000133000233000433000533000733000833000933000a33000b33000c33000d33000e33000f3300113300123300133300143300152d2e01022581f8acd19181cf959a1281f8acd19181cf951a81f8acd19181cf1581f8acd191810f082209240a220b230c240d250e230f23102311231223132314181b1c28171615400003290349483403350222231d4a484848482b50512a632223524100034200004322602261222704634848222862482864286548482228246628226723286828692322700048482371004848361c0037001a0031183119311b311d311e311f312023221e312131223123312431253126312731283129312a312b312c312d312e312f447825225314225427042455220824564c4d4b0222382124391c0081e80780046a6f686e2281d00f23241f880003420001892224902291922494249593a0a1a2a3a4a5a6a7a8a9aaabacadae24af3a00003b003c003d816472064e014f012a57000823810858235b235a2359b03139330039b1b200b322c01a23c1001a2323c21a23c3233e233f8120af06002a494905002a49490700b53a03",
6: "062004010002b7a60c26050242420c68656c6c6f20776f726c6421070123456789abcd208dae2087fbba51304eb02b91f656948397a7946390e8cb70fc9ea4d95f92251d047465737400320032013202320380021234292929292b0431003101310231043105310731083109310a310b310c310d310e310f3111311231133114311533000033000133000233000433000533000733000833000933000a33000b33000c33000d33000e33000f3300113300123300133300143300152d2e01022581f8acd19181cf959a1281f8acd19181cf951a81f8acd19181cf1581f8acd191810f082209240a220b230c240d250e230f23102311231223132314181b1c28171615400003290349483403350222231d4a484848482b50512a632223524100034200004322602261222704634848222862482864286548482228246628226723286828692322700048482371004848361c0037001a0031183119311b311d311e311f312023221e312131223123312431253126312731283129312a312b312c312d312e312f447825225314225427042455220824564c4d4b0222382124391c0081e80780046a6f686e2281d00f23241f880003420001892224902291922494249593a0a1a2a3a4a5a6a7a8a9aaabacadae24af3a00003b003c003d816472064e014f012a57000823810858235b235a2359b03139330039b1b200b322c01a23c1001a2323c21a23c3233e233f8120af06002a494905002a49490700b53a03",
6: "062004010002b7a60c26050242420c68656c6c6f20776f726c6421070123456789abcd208dae2087fbba51304eb02b91f656948397a7946390e8cb70fc9ea4d95f92251d047465737400320032013202320380021234292929292b0431003101310231043105310731083109310a310b310c310d310e310f3111311231133114311533000033000133000233000433000533000733000833000933000a33000b33000c33000d33000e33000f3300113300123300133300143300152d2e01022581f8acd19181cf959a1281f8acd19181cf951a81f8acd19181cf1581f8acd191810f082209240a220b230c240d250e230f23102311231223132314181b1c28171615400003290349483403350222231d4a484848482b50512a632223524100034200004322602261222704634848222862482864286548482228246628226723286828692322700048482371004848361c0037001a0031183119311b311d311e311f312023221e312131223123312431253126312731283129312a312b312c312d312e312f447825225314225427042455220824564c4d4b0222382124391c0081e80780046a6f686e2281d00f23241f880003420001892224902291922494249593a0a1a2a3a4a5a6a7a8a9aaabacadae24af3a00003b003c003d816472064e014f012a57000823810858235b235a2359b03139330039b1b200b322c01a23c1001a2323c21a23c3233e233f8120af06002a494905002a49490700b53a03b6",
}

func pseudoOp(opcode string) bool {
Expand Down
7 changes: 4 additions & 3 deletions data/transactions/logic/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,10 @@ var opDocByName = map[string]string{
"b~": "X with all bits inverted",

"log": "write bytes to log state of the current application",
"itxn_begin": "begin preparation of a new inner transaction",
"itxn_begin": "begin preparation of a new inner transaction in a new transaction group",
"itxn_next": "begin preparation of a new inner transaction in the same transaction group",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An expanded description for this opcode would be helpful to explain when it's useful.

I.e., all inner transactions are atomic in the sense that if one fails, the entire top-level transaction fails. However, explicitly putting transactions into groups enables fee pooling and setting up the environment for inner transaction application calls.

"itxn_field": "set field F of the current inner transaction to X",
"itxn_submit": "execute the current inner transaction. Fail if 16 inner transactions have already been executed, or if the transaction itself fails.",
"itxn_submit": "execute the current inner transaction group. Fail if executing this group would exceed 16 total inner transactions, or if any transaction in the group fails.",
}

// OpDoc returns a description of the op
Expand Down Expand Up @@ -300,7 +301,7 @@ var OpGroups = map[string][]string{
"Loading Values": {"intcblock", "intc", "intc_0", "intc_1", "intc_2", "intc_3", "pushint", "bytecblock", "bytec", "bytec_0", "bytec_1", "bytec_2", "bytec_3", "pushbytes", "bzero", "arg", "arg_0", "arg_1", "arg_2", "arg_3", "args", "txn", "gtxn", "txna", "txnas", "gtxna", "gtxnas", "gtxns", "gtxnsa", "gtxnsas", "global", "load", "loads", "store", "stores", "gload", "gloads", "gaid", "gaids"},
"Flow Control": {"err", "bnz", "bz", "b", "return", "pop", "dup", "dup2", "dig", "cover", "uncover", "swap", "select", "assert", "callsub", "retsub"},
"State Access": {"balance", "min_balance", "app_opted_in", "app_local_get", "app_local_get_ex", "app_global_get", "app_global_get_ex", "app_local_put", "app_global_put", "app_local_del", "app_global_del", "asset_holding_get", "asset_params_get", "app_params_get", "log"},
"Inner Transactions": {"itxn_begin", "itxn_field", "itxn_submit", "itxn", "itxna"},
"Inner Transactions": {"itxn_begin", "itxn_next", "itxn_field", "itxn_submit", "itxn", "itxna"},
}

// OpCost indicates the cost of an operation over the range of
Expand Down
173 changes: 107 additions & 66 deletions data/transactions/logic/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,8 @@ type EvalContext struct {
version uint64
scratch scratchSpace

subtxn *transactions.SignedTxn // place to build for itxn_submit
// The transactions Performed() and their effects
subtxns []transactions.SignedTxn // place to build for itxn_submit
// Previous transactions Performed() and their effects
InnerTxns []transactions.SignedTxnWithAD

cost int // cost incurred so far
Expand Down Expand Up @@ -3676,33 +3676,68 @@ func authorizedSender(cx *EvalContext, addr basics.Address) bool {
return appAddr == authorizer
}

func opTxBegin(cx *EvalContext) {
if cx.subtxn != nil {
cx.err = errors.New("itxn_begin without itxn_submit")
return
}
// Start fresh
cx.subtxn = &transactions.SignedTxn{}
// Fill in defaults.
// addInnerTxn appends a fresh SignedTxn to subtxns, populated with reasonable
// defaults.
func addInnerTxn(cx *EvalContext) error {
addr, err := cx.getApplicationAddress()
if err != nil {
cx.err = err
return
return err
}

fee := cx.Proto.MinTxnFee
if cx.FeeCredit != nil {
// Use credit to shrink the fee, but don't change FeeCredit
// here, because they might never itxn_submit, or they might
// change the fee. Do it in itxn_submit.
fee = basics.SubSaturate(fee, *cx.FeeCredit)
// For compatibility with v5, in which failures only occurred in the submit,
// we only fail here if we are OVER the MaxInnerTransactions limit. Thus
// this allows construction of one more Inner than is actually allowed, and
// will fail in submit. (But we do want the check here, so this can't become
// unbounded.) The MaxTxGroupSize check can be, and is, precise.
if len(cx.InnerTxns)+len(cx.subtxns) > cx.Proto.MaxInnerTransactions ||
len(cx.subtxns) >= cx.Proto.MaxTxGroupSize {
return errors.New("attempt to create too many inner transactions")
}

stxn := transactions.SignedTxn{}

groupFee := basics.MulSaturate(cx.Proto.MinTxnFee, uint64(len(cx.subtxns)+1))
groupPaid := uint64(0)
for _, ptxn := range cx.subtxns {
groupPaid = basics.AddSaturate(groupPaid, ptxn.Txn.Fee.Raw)
}

fee := uint64(0)
if groupPaid < groupFee {
fee = groupFee - groupPaid

if cx.FeeCredit != nil {
// Use credit to shrink the default populated fee, but don't change
// FeeCredit here, because they might never itxn_submit, or they
// might change the fee. Do it in itxn_submit.
fee = basics.SubSaturate(fee, *cx.FeeCredit)
}
}
cx.subtxn.Txn.Header = transactions.Header{
Sender: addr, // Default, to simplify usage

stxn.Txn.Header = transactions.Header{
Sender: addr,
Fee: basics.MicroAlgos{Raw: fee},
FirstValid: cx.Txn.Txn.FirstValid,
LastValid: cx.Txn.Txn.LastValid,
}
cx.subtxns = append(cx.subtxns, stxn)
return nil
}

func opTxBegin(cx *EvalContext) {
if len(cx.subtxns) > 0 {
cx.err = errors.New("itxn_begin without itxn_submit")
return
}
cx.err = addInnerTxn(cx)
}

func opTxNext(cx *EvalContext) {
if len(cx.subtxns) == 0 {
cx.err = errors.New("itxn_next without itxn_begin")
return
}
cx.err = addInnerTxn(cx)
}

// availableAccount is used instead of accountReference for more recent opcodes
Expand Down Expand Up @@ -3884,7 +3919,8 @@ func (cx *EvalContext) stackIntoTxnField(sv stackValue, fs txnFieldSpec, txn *tr
}

func opTxField(cx *EvalContext) {
if cx.subtxn == nil {
itx := len(cx.subtxns) - 1
if itx < 0 {
cx.err = errors.New("itxn_field without itxn_begin")
return
}
Expand All @@ -3896,7 +3932,7 @@ func opTxField(cx *EvalContext) {
return
}
sv := cx.stack[last]
cx.err = cx.stackIntoTxnField(sv, fs, &cx.subtxn.Txn)
cx.err = cx.stackIntoTxnField(sv, fs, &cx.subtxns[itx].Txn)
cx.stack = cx.stack[:last] // pop
}

Expand All @@ -3906,64 +3942,69 @@ func opTxSubmit(cx *EvalContext) {
return
}

if cx.subtxn == nil {
cx.err = errors.New("itxn_submit without itxn_begin")
// Should never trigger, since itxn_next checks these too.
if len(cx.InnerTxns)+len(cx.subtxns) > cx.Proto.MaxInnerTransactions ||
len(cx.subtxns) > cx.Proto.MaxTxGroupSize {
cx.err = errors.New("too many inner transactions")
return
}

if len(cx.InnerTxns) >= cx.Proto.MaxInnerTransactions {
cx.err = errors.New("itxn_submit with MaxInnerTransactions")
if len(cx.subtxns) == 0 {
cx.err = errors.New("itxn_submit without itxn_begin")
return
}

// The goal is to follow the same invariants used by the
// transaction pool. Namely that any transaction that makes it
// to Perform (which is equivalent to eval.applyTransaction)
// is authorized, and WellFormed.
if !authorizedSender(cx, cx.subtxn.Txn.Sender) {
cx.err = fmt.Errorf("unauthorized")
return
// Check fees across the group first. Allows fee pooling in inner groups.
groupFee := basics.MulSaturate(cx.Proto.MinTxnFee, uint64(len(cx.subtxns)))
groupPaid := uint64(0)
for _, ptxn := range cx.subtxns {
groupPaid = basics.AddSaturate(groupPaid, ptxn.Txn.Fee.Raw)
}

// Recall that WellFormed does not care about individual
// transaction fees because of fee pooling. So we check below.
cx.err = cx.subtxn.Txn.WellFormed(*cx.Specials, *cx.Proto)
if cx.err != nil {
return
}

paid := cx.subtxn.Txn.Fee.Raw
if paid >= cx.Proto.MinTxnFee {
// Over paying - accumulate into FeeCredit
overpaid := paid - cx.Proto.MinTxnFee
if groupPaid < groupFee {
// See if the FeeCredit is enough to cover the shortfall
shortfall := groupFee - groupPaid
if cx.FeeCredit == nil || *cx.FeeCredit < shortfall {
cx.err = fmt.Errorf("fee too small %#v", cx.subtxns)
return
}
*cx.FeeCredit -= shortfall
} else {
overpay := groupPaid - groupFee
if cx.FeeCredit == nil {
cx.FeeCredit = new(uint64)
}
*cx.FeeCredit = basics.AddSaturate(*cx.FeeCredit, overpaid)
} else {
underpaid := cx.Proto.MinTxnFee - paid
// Try to pay with FeeCredit, else fail.
if cx.FeeCredit != nil && *cx.FeeCredit >= underpaid {
*cx.FeeCredit -= underpaid
} else {
// We allow changing the fee. One pattern might be for an
// app to unilaterally set its Fee to 0. The idea would be
// that other transactions were supposed to overpay.
cx.err = fmt.Errorf("fee too small")
*cx.FeeCredit = basics.AddSaturate(*cx.FeeCredit, overpay)
}

for itx := range cx.subtxns {
// The goal is to follow the same invariants used by the
// transaction pool. Namely that any transaction that makes it
// to Perform (which is equivalent to eval.applyTransaction)
// is authorized, and WellFormed.
if !authorizedSender(cx, cx.subtxns[itx].Txn.Sender) {
cx.err = fmt.Errorf("unauthorized")
return
}
}

ad, err := cx.Ledger.Perform(&cx.subtxn.Txn, *cx.Specials)
if err != nil {
cx.err = err
return
// Recall that WellFormed does not care about individual
// transaction fees because of fee pooling. So we check below.
cx.err = cx.subtxns[itx].Txn.WellFormed(*cx.Specials, *cx.Proto)
if cx.err != nil {
return
}

ad, err := cx.Ledger.Perform(&cx.subtxns[itx].Txn, *cx.Specials)
if err != nil {
cx.err = err
return
}

cx.InnerTxns = append(cx.InnerTxns, transactions.SignedTxnWithAD{
SignedTxn: cx.subtxns[itx],
ApplyData: ad,
})
}
cx.InnerTxns = append(cx.InnerTxns, transactions.SignedTxnWithAD{
SignedTxn: *cx.subtxn,
ApplyData: ad,
})
cx.subtxn = nil
cx.subtxns = nil
}

// PcDetails return PC and disassembled instructions at PC up to 2 opcodes back
Expand Down
64 changes: 63 additions & 1 deletion data/transactions/logic/evalAppTxn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ func TestNumInner(t *testing.T) {
testApp(t, pay+pay+pay+";int 1", ep)
testApp(t, pay+pay+pay+pay+";int 1", ep)
// In the sample proto, MaxInnerTransactions = 4
testApp(t, pay+pay+pay+pay+pay+";int 1", ep, "itxn_submit with MaxInnerTransactions")
testApp(t, pay+pay+pay+pay+pay+";int 1", ep, "too many inner transactions")
}

func TestAssetCreate(t *testing.T) {
Expand Down Expand Up @@ -518,3 +518,65 @@ func TestFieldSetting(t *testing.T) {
testApp(t, "itxn_begin; int 13; bzero; itxn_field ConfigAssetName; int 1", ep,
"value is too long")
}

func TestInnerGroup(t *testing.T) {
ep, ledger := makeSampleEnv()
ledger.NewApp(ep.Txn.Txn.Receiver, 888, basics.AppParams{})
// Need both fees and both payments
ledger.NewAccount(ledger.ApplicationID().Address(), 999+2*defaultEvalProto().MinTxnFee)
pay := `
int pay; itxn_field TypeEnum;
int 500; itxn_field Amount;
txn Sender; itxn_field Receiver;
`
testApp(t, "itxn_begin"+pay+"itxn_next"+pay+"itxn_submit; int 1", ep,
"insufficient balance")

// NewAccount overwrites the existing balance
ledger.NewAccount(ledger.ApplicationID().Address(), 1000+2*defaultEvalProto().MinTxnFee)
testApp(t, "itxn_begin"+pay+"itxn_next"+pay+"itxn_submit; int 1", ep)
}

func TestInnerFeePooling(t *testing.T) {
ep, ledger := makeSampleEnv()
ledger.NewApp(ep.Txn.Txn.Receiver, 888, basics.AppParams{})
ledger.NewAccount(ledger.ApplicationID().Address(), 50_000)
pay := `
int pay; itxn_field TypeEnum;
int 500; itxn_field Amount;
txn Sender; itxn_field Receiver;
`
// Force the first fee to 3, but the second will default to 2*fee-3 = 2002-3
testApp(t, "itxn_begin"+
pay+
"int 3; itxn_field Fee;"+
"itxn_next"+
pay+
"itxn_submit; itxn Fee; int 1999; ==", ep)

// Same first, but force the second too low
testApp(t, "itxn_begin"+
pay+
"int 3; itxn_field Fee;"+
"itxn_next"+
pay+
"int 1998; itxn_field Fee;"+
"itxn_submit; int 1", ep, "fee too small")

// Overpay in first itxn, the second will default to less
testApp(t, "itxn_begin"+
pay+
"int 2000; itxn_field Fee;"+
"itxn_next"+
pay+
"itxn_submit; itxn Fee; int 2; ==", ep)

// Same first, but force the second too low
testApp(t, "itxn_begin"+
pay+
"int 2000; itxn_field Fee;"+
"itxn_next"+
pay+
"int 1; itxn_field Fee;"+
"itxn_submit; itxn Fee; int 1", ep, "fee too small")
}
Loading