Skip to content

Commit

Permalink
Checkpoint with new approach, needs v9 -> v8 tests
Browse files Browse the repository at this point in the history 
Does not yet have the extra check to ensure v9 can't expose new
resources by putting accounts from one tx and assets from another into
static arrays of a called v8 app.
  • Loading branch information
@jannotti
jannotti committed Jan 26, 2023
1 parent 0a06a9e commit 72b3af4
Show file tree
Hide file tree
Showing 6 changed files with 519 additions and 316 deletions.
14 changes: 0 additions & 14 deletions data/basics/userBalance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,20 +343,6 @@ type CreatableLocator struct {
Index CreatableIndex
}

// HoldingLocator stores enough information to name a "Holding" the balance of
// an ASA for a particular account.
type HoldingLocator struct {
Address Address
Index AssetIndex
}

// LocalsLocator stores enough information to find the Local State that an
// account has with a particular app.
type LocalsLocator struct {
Address Address
Index AppIndex
}

// AssetHolding describes an asset held by an account.
type AssetHolding struct {
_struct struct{} `codec:",omitempty,omitemptyarray"`
Expand Down
245 changes: 0 additions & 245 deletions data/transactions/logic/blackbox_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,248 +97,3 @@ func TestNewAppEvalParams(t *testing.T) {
}
}
}

// TestAppSharing confirms that as of v9, assets can be accessed across
// groups, but that before then, they could not.
func TestAppSharing(t *testing.T) {
partitiontest.PartitionTest(t)
t.Parallel()

// Create some sample transactions. The main reason this a blackbox test
// (_test package) is to have access to txntest.
appl0 := txntest.Txn{
Type: protocol.ApplicationCallTx,
Sender: basics.Address{1, 2, 3, 4},
ForeignApps: []basics.AppIndex{500},
}

appl1 := txntest.Txn{
Type: protocol.ApplicationCallTx,
Sender: basics.Address{4, 3, 2, 1},
}

appl2 := txntest.Txn{
Type: protocol.ApplicationCallTx,
Sender: basics.Address{1, 2, 3, 4},
}

getSchema := `
int 500
app_params_get AppGlobalNumByteSlice
!; assert; pop; int 1
`
sources := []string{getSchema, getSchema}
// In v8, the first tx can read app params of 500, because it's in its
// foreign array, but the second can't
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl1), 8, nil,
logic.NewExpect(1, "invalid App reference 500"))
// In v9, the second can, because the first can.
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl1), 9, nil)

getLocalEx := `
int 0 // Sender
int 500
byte "some-key"
app_local_get_ex
pop; pop; int 1
`

sources = []string{getLocalEx, getLocalEx}
// In contrast, here there's no help from v9, because the second tx is
// reading the locals for a different account.

// app_local_get* requires the address and the app exist, else the program fails
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl1), 8, nil,
logic.NewExpect(0, "no account"))

_, _, ledger := logic.MakeSampleEnv()
ledger.NewAccount(appl0.Sender, 100_000)
ledger.NewApp(appl0.Sender, 500, basics.AppParams{})
ledger.NewLocals(appl0.Sender, 500) // opt in
// Now txn0 passes, but txn1 has an error because it can't see app 500
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl1), 9, ledger,
logic.NewExpect(1, "invalid Local State access"))

// But it's ok in appl2, because appl2 uses the same Sender, even though the
// foreign-app is not repeated in appl2 so the holding being accessed is is
// the one from tx0.
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl2), 9, ledger)
}

// TestAssetSharing confirms that as of v9, assets can be accessed across
// groups, but that before then, they could not.
func TestAssetSharing(t *testing.T) {
partitiontest.PartitionTest(t)
t.Parallel()

// Create some sample transactions. The main reason this a blackbox test
// (_test package) is to have access to txntest.
appl0 := txntest.Txn{
Type: protocol.ApplicationCallTx,
Sender: basics.Address{1, 2, 3, 4},
ForeignAssets: []basics.AssetIndex{400},
}

appl1 := txntest.Txn{
Type: protocol.ApplicationCallTx,
Sender: basics.Address{4, 3, 2, 1},
}

appl2 := txntest.Txn{
Type: protocol.ApplicationCallTx,
Sender: basics.Address{1, 2, 3, 4},
}

getTotal := `
int 400
asset_params_get AssetTotal
pop; pop; int 1
`
sources := []string{getTotal, getTotal}
// In v8, the first tx can read asset 400, because it's in its foreign arry,
// but the second can't
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl1), 8, nil,
logic.NewExpect(1, "invalid Asset reference 400"))
// In v9, the second can, because the first can.
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl1), 9, nil)

getBalance := `
int 0
int 400
asset_holding_get AssetBalance
pop; pop; int 1
`

sources = []string{getBalance, getBalance}
// In contrast, here there's no help from v9, because the second tx is
// reading a holding for a different account.
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl1), 8, nil,
logic.NewExpect(1, "invalid Asset reference 400"))
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl1), 9, nil,
logic.NewExpect(1, "invalid Holding access"))
// But it's ok in appl2, because the same account is used, even though the
// foreign-asset is not repeated in appl2.
logic.TestApps(t, sources, txntest.SignedTxns(&appl0, &appl2), 9, nil)
}

// TestOtherTxSharing tests resource sharing across other kinds of transactions besides appl.
func TestOtherTxSharing(t *testing.T) {
partitiontest.PartitionTest(t)
t.Parallel()

_, _, ledger := logic.MakeSampleEnv()

senderAcct := basics.Address{1, 2, 3, 4, 5, 6, 1}
ledger.NewAccount(senderAcct, 2001)
senderBalance := "txn ApplicationArgs 0; balance; int 2001; =="

receiverAcct := basics.Address{1, 2, 3, 4, 5, 6, 2}
ledger.NewAccount(receiverAcct, 2002)
receiverBalance := "txn ApplicationArgs 0; balance; int 2002; =="

otherAcct := basics.Address{1, 2, 3, 4, 5, 6, 3}
ledger.NewAccount(otherAcct, 2003)
otherBalance := "txn ApplicationArgs 0; balance; int 2003; =="

appl := txntest.Txn{
Type: protocol.ApplicationCallTx,
ApplicationArgs: [][]byte{senderAcct[:]},
}

keyreg := txntest.Txn{
Type: protocol.KeyRegistrationTx,
Sender: senderAcct,
}
pay := txntest.Txn{
Type: protocol.PaymentTx,
Sender: senderAcct,
Receiver: receiverAcct,
}
acfg := txntest.Txn{
Type: protocol.AssetConfigTx,
Sender: senderAcct,
}
axfer := txntest.Txn{
Type: protocol.AssetTransferTx,
XferAsset: 100, // must be < 256, later code assumes it fits in a byte
Sender: senderAcct,
AssetReceiver: receiverAcct,
AssetSender: otherAcct,
}
afrz := txntest.Txn{
Type: protocol.AssetFreezeTx,
Sender: senderAcct,
FreezeAccount: otherAcct,
}

sources := []string{"", senderBalance}
rsources := []string{senderBalance, ""}
for _, send := range []txntest.Txn{keyreg, pay, acfg, axfer, afrz} {
logic.TestApps(t, sources, txntest.SignedTxns(&send, &appl), 9, ledger)
logic.TestApps(t, rsources, txntest.SignedTxns(&appl, &send), 9, ledger)

logic.TestApps(t, sources, txntest.SignedTxns(&send, &appl), 8, ledger,
logic.NewExpect(1, "invalid Account reference"))
logic.TestApps(t, rsources, txntest.SignedTxns(&appl, &send), 8, ledger,
logic.NewExpect(0, "invalid Account reference"))
}

holdingAccess := `
txn ApplicationArgs 0
txn ApplicationArgs 1; btoi
asset_holding_get AssetBalance
pop; pop
`
sources = []string{"", holdingAccess}
rsources = []string{holdingAccess, ""}

t.Run("keyreg", func(t *testing.T) {
appl.ApplicationArgs = [][]byte{senderAcct[:], {200}}
logic.TestApps(t, sources, txntest.SignedTxns(&keyreg, &appl), 9, ledger,
logic.NewExpect(1, "invalid Asset reference 200"))
withRef := appl
withRef.ForeignAssets = []basics.AssetIndex{200}
logic.TestApps(t, sources, txntest.SignedTxns(&keyreg, &withRef), 9, ledger,
logic.NewExpect(1, "invalid Holding access "+senderAcct.String()))
})
t.Run("pay", func(t *testing.T) {
// The receiver is available for algo balance reading
appl.ApplicationArgs = [][]byte{receiverAcct[:]}
logic.TestApps(t, []string{"", receiverBalance}, txntest.SignedTxns(&pay, &appl), 9, ledger)

// The other account is not (it's not even in the pay txn)
appl.ApplicationArgs = [][]byte{otherAcct[:]}
logic.TestApps(t, []string{"", otherBalance}, txntest.SignedTxns(&pay, &appl), 9, ledger,
logic.NewExpect(1, "invalid Account reference "+otherAcct.String()))

// The other account becomes accessible because used in CloseRemainderTo
withClose := pay
withClose.CloseRemainderTo = otherAcct
logic.TestApps(t, []string{"", otherBalance}, txntest.SignedTxns(&withClose, &appl), 9, ledger)
})

t.Run("acfg", func(t *testing.T) {
})

t.Run("axfer", func(t *testing.T) {
// The receiver is NOT available for algo balance reading (only the holding for the asa)
appl.ApplicationArgs = [][]byte{receiverAcct[:]}
logic.TestApps(t, []string{"", receiverBalance}, txntest.SignedTxns(&axfer, &appl), 9, ledger,
logic.NewExpect(1, "invalid Account reference "+receiverAcct.String()))

appl.ApplicationArgs = [][]byte{receiverAcct[:], {byte(axfer.XferAsset)}}
/*
logic.TestApps(t, []string{"", holdingAccess}, txntest.SignedTxns(&axfer, &appl), 9, ledger)
// The other account becomes accessible because used in CloseRemainderTo (for asa, not algo)
withClose := axfer
withClose.AssetCloseTo = otherAcct
logic.TestApps(t, []string{"", otherBalance}, txntest.SignedTxns(&withClose, &appl), 9, ledger,
logic.NewExpect(1, "bad"))
logic.TestApps(t, []string{"", holdingAccess}, txntest.SignedTxns(&withClose, &appl), 9, ledger)
*/
})

t.Run("afrz", func(t *testing.T) {
})
}
93 changes: 55 additions & 38 deletions data/transactions/logic/eval.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,26 +380,12 @@ func (ep *EvalParams) computeAvailability() *resources {
sharedAccounts: make(map[basics.Address]struct{}),
sharedAsas: make(map[basics.AssetIndex]struct{}),
sharedApps: make(map[basics.AppIndex]struct{}),
sharedHoldings: make(map[basics.HoldingLocator]struct{}),
sharedLocals: make(map[basics.LocalsLocator]struct{}),
sharedHoldings: make(map[ledgercore.AccountAsset]struct{}),
sharedLocals: make(map[ledgercore.AccountApp]struct{}),
boxes: make(map[boxRef]bool),
}
for i := range ep.TxnGroup {
tx := &ep.TxnGroup[i]
switch tx.Txn.Type {
case protocol.PaymentTx:
available.fillPayment(&tx.Txn.Header, &tx.Txn.PaymentTxnFields)
case protocol.KeyRegistrationTx:
available.fillKeyRegistration(&tx.Txn.Header)
case protocol.AssetConfigTx:
available.fillAssetConfig(&tx.Txn.Header, &tx.Txn.AssetConfigTxnFields)
case protocol.AssetTransferTx:
available.fillAssetTransfer(&tx.Txn.Header, &tx.Txn.AssetTransferTxnFields)
case protocol.AssetFreezeTx:
available.fillAssetFreeze(&tx.Txn.Header, &tx.Txn.AssetFreezeTxnFields)
case protocol.ApplicationCallTx:
available.fillApplicationCall(ep, &tx.Txn.Header, &tx.Txn.ApplicationCallTxnFields)
}
available.fill(&ep.TxnGroup[i].Txn, ep)
}
return available
}
Expand Down Expand Up @@ -4433,22 +4419,59 @@ func (cx *EvalContext) assetReference(ref uint64, foreign bool) (basics.AssetInd

}

func (cx *EvalContext) holdingReference(account stackValue, ref uint64) (addr basics.Address, asset basics.AssetIndex, err error) {
addr, _, err = cx.accountReference(account)
if err != nil {
return
func (cx *EvalContext) holdingReference(account stackValue, ref uint64) (basics.Address, basics.AssetIndex, error) {
if cx.version >= resourceSharingVersion {
var addr basics.Address
var err error
if account.Bytes != nil {
addr, err = account.address()
} else {
addr, err = cx.txn.Txn.AddressByIndex(account.Uint, cx.txn.Txn.Sender)
}
if err != nil {
return basics.Address{}, basics.AssetIndex(0), err
}
aid := basics.AssetIndex(ref)
if cx.availableHolding(addr, aid) {
return addr, aid, nil
}
if ref < uint64(len(cx.txn.Txn.ForeignAssets)) {
aid := cx.txn.Txn.ForeignAssets[ref]
if cx.availableHolding(addr, aid) {
return addr, aid, nil
}
}

// Do some extra lookups to give a more concise err. Whenever a holding
// is available, its account and asset must be as well (but not vice
// versa, anymore). So, if (only) one of them is not available, yell
// about it, specifically.

_, _, acctErr := cx.accountReference(account)
_, assetErr := cx.assetReference(ref, false)
switch {
case acctErr != nil && assetErr == nil:
err = acctErr
case acctErr == nil && assetErr != nil:
err = assetErr
default:
err = fmt.Errorf("invalid Holding access %s x %d", addr, aid)
}

return basics.Address{}, basics.AssetIndex(0), err
}
asset, err = cx.assetReference(ref, false)

// Pre group resource sharing, the rule is just that account and asset are
// each available.
addr, _, err := cx.accountReference(account)
if err != nil {
return
return basics.Address{}, basics.AssetIndex(0), err
}

if !cx.availableHolding(addr, asset) {
err = fmt.Errorf("invalid Holding access %s x %d", addr, asset)
return
asset, err := cx.assetReference(ref, false)
if err != nil {
return basics.Address{}, basics.AssetIndex(0), err
}

return
return addr, asset, nil
}

func opAssetHoldingGet(cx *EvalContext) error {
Expand Down Expand Up @@ -4803,15 +4826,9 @@ func (cx *EvalContext) availableApp(aid basics.AppIndex) bool {
return false
}

// availableHolding is an additional check, required since
// resourceSharingVersion. It will work in previous versions too, since
// cx.available will be populated to make it work. But it must protect for <
// directRefEnabledVersion, because unnamed holdings could be read then.
// availableHolding checks if a holding is available under the txgroup sharing rules
func (cx *EvalContext) availableHolding(addr basics.Address, ai basics.AssetIndex) bool {
if cx.version < directRefEnabledVersion {
return true
}
if _, ok := cx.available.sharedHoldings[basics.HoldingLocator{Address: addr, Index: ai}]; ok {
if _, ok := cx.available.sharedHoldings[ledgercore.AccountAsset{Address: addr, Asset: ai}]; ok {
return true
}
// All holdings of created assets are available
Expand All @@ -4831,7 +4848,7 @@ func (cx *EvalContext) availableLocals(addr basics.Address, ai basics.AppIndex)
if cx.version < directRefEnabledVersion {
return true
}
if _, ok := cx.available.sharedLocals[basics.LocalsLocator{Address: addr, Index: ai}]; ok {
if _, ok := cx.available.sharedLocals[ledgercore.AccountApp{Address: addr, App: ai}]; ok {
return true
}
// All locals of created apps are available
Expand Down

0 comments on commit 72b3af4

Please sign in to comment.