diff --git a/data/transactions/asset.go b/data/transactions/asset.go index 93b6af840e..dc76d7bb8c 100644 --- a/data/transactions/asset.go +++ b/data/transactions/asset.go @@ -17,6 +17,10 @@ package transactions import ( + "errors" + "fmt" + + "github.com/algorand/go-algorand/config" "github.com/algorand/go-algorand/data/basics" ) @@ -75,3 +79,47 @@ type AssetFreezeTxnFields struct { // AssetFrozen is the new frozen value. AssetFrozen bool `codec:"afrz"` } + +func (ac AssetConfigTxnFields) wellFormed(proto config.ConsensusParams) error { + if len(ac.AssetParams.AssetName) > proto.MaxAssetNameBytes { + return fmt.Errorf("transaction asset name too big: %d > %d", len(ac.AssetParams.AssetName), proto.MaxAssetNameBytes) + } + + if len(ac.AssetParams.UnitName) > proto.MaxAssetUnitNameBytes { + return fmt.Errorf("transaction asset unit name too big: %d > %d", len(ac.AssetParams.UnitName), proto.MaxAssetUnitNameBytes) + } + + if len(ac.AssetParams.URL) > proto.MaxAssetURLBytes { + return fmt.Errorf("transaction asset url too big: %d > %d", len(ac.AssetParams.URL), proto.MaxAssetURLBytes) + } + + if ac.AssetParams.Decimals > proto.MaxAssetDecimals { + return fmt.Errorf("transaction asset decimals is too high (max is %d)", proto.MaxAssetDecimals) + } + + return nil +} + +func (ax AssetTransferTxnFields) wellFormed() error { + if ax.XferAsset == 0 && ax.AssetAmount != 0 { + return errors.New("asset ID cannot be zero") + } + + if !ax.AssetSender.IsZero() && !ax.AssetCloseTo.IsZero() { + return errors.New("cannot close asset by clawback") + } + + return nil +} + +func (af AssetFreezeTxnFields) wellFormed() error { + if af.FreezeAsset == 0 { + return errors.New("asset ID cannot be zero") + } + + if af.FreezeAccount.IsZero() { + return errors.New("freeze account cannot be empty") + } + + return nil +} diff --git a/data/transactions/logic/evalAppTxn_test.go b/data/transactions/logic/evalAppTxn_test.go index 3008d517d6..21dbc52087 100644 --- a/data/transactions/logic/evalAppTxn_test.go +++ b/data/transactions/logic/evalAppTxn_test.go @@ -74,9 +74,7 @@ func TestCurrentInnerTypes(t *testing.T) { TestApp(t, "itxn_begin; int axfer; itxn_field TypeEnum; itxn_submit; int 1;", ep, "insufficient balance") TestApp(t, "itxn_begin; byte \"acfg\"; itxn_field Type; itxn_submit; int 1;", ep, "insufficient balance") - TestApp(t, "itxn_begin; byte \"afrz\"; itxn_field Type; itxn_submit; int 1;", ep, "insufficient balance") TestApp(t, "itxn_begin; int acfg; itxn_field TypeEnum; itxn_submit; int 1;", ep, "insufficient balance") - TestApp(t, "itxn_begin; int afrz; itxn_field TypeEnum; itxn_submit; int 1;", ep, "insufficient balance") // allowed since v6 TestApp(t, "itxn_begin; byte \"keyreg\"; itxn_field Type; itxn_submit; int 1;", ep, "insufficient balance") @@ -431,6 +429,13 @@ func TestAppAxfer(t *testing.T) { partitiontest.PartitionTest(t) t.Parallel() + closeWithClawback := ` + itxn_begin + int axfer ; itxn_field TypeEnum + txn Sender ; itxn_field AssetSender + txn Sender ; itxn_field AssetCloseTo + itxn_submit +` axfer := ` itxn_begin int 77 @@ -450,6 +455,8 @@ func TestAppAxfer(t *testing.T) { TestApp(t, source, ep, problem...) } + test(closeWithClawback, "cannot close asset by clawback") + ledger.NewApp(tx.Receiver, 888, basics.AppParams{}) ledger.NewAsset(tx.Receiver, 777, basics.AssetParams{}) // not in foreign-assets of sample ledger.NewAsset(tx.Receiver, 77, basics.AssetParams{}) // in foreign-assets of sample @@ -461,6 +468,14 @@ func TestAppAxfer(t *testing.T) { "assert failed") // app account not opted in ledger.NewAccount(appAddr(888), 10000) // plenty for fees + + // It should be possible to send 0 amount of an asset (existing + // or not) to any account but ourself. Regardless of being opted in + test("global CurrentApplicationAddress; txn Accounts 1; int 0" + axfer + "int 1") + holding, err := ledger.AssetHolding(appAddr(888), 77) + require.ErrorContains(t, err, "no asset 77 for account") + require.Equal(t, uint64(0), holding.Amount) + ledger.NewHolding(appAddr(888), 77, 3000, false) test("global CurrentApplicationAddress; int 77; asset_holding_get AssetBalance; assert; int 3000; ==;") @@ -494,7 +509,7 @@ func TestAppAxfer(t *testing.T) { // is going to fail anyway, but to keep the behavior consistent, v9 // allows the zero asset (and zero account) in `requireHolding`. test("global CurrentApplicationAddress; txn Accounts 1; int 100"+noid+"int 1", - fmt.Sprintf("Sender (%s) not opted in to 0", appAddr(888))) + "asset ID cannot be zero") test("global CurrentApplicationAddress; txn Accounts 1; int 100" + axfer + "int 1") @@ -775,14 +790,7 @@ func TestAssetFreeze(t *testing.T) { int 5000 == ` - // v5 added inners - TestLogicRange(t, 5, 0, func(t *testing.T, ep *EvalParams, tx *transactions.Transaction, ledger *Ledger) { - ledger.NewApp(tx.Receiver, 888, basics.AppParams{}) - // Give it enough for fees. Recall that we don't check min balance at this level. - ledger.NewAccount(appAddr(888), 12*MakeTestProto().MinTxnFee) - TestApp(t, create, ep) - - freeze := ` + freeze := ` itxn_begin int afrz ; itxn_field TypeEnum int 5000 ; itxn_field FreezeAsset @@ -791,6 +799,24 @@ func TestAssetFreeze(t *testing.T) { itxn_submit int 1 ` + missingFreezeAccount := ` + itxn_begin + int afrz ; itxn_field TypeEnum + int 5000 ; itxn_field FreezeAsset + itxn_submit +` + missingAssetID := ` + itxn_begin + int afrz ; itxn_field TypeEnum + itxn_submit +` + // v5 added inners + TestLogicRange(t, 5, 0, func(t *testing.T, ep *EvalParams, tx *transactions.Transaction, ledger *Ledger) { + ledger.NewApp(tx.Receiver, 888, basics.AppParams{}) + // Give it enough for fees. Recall that we don't check min balance at this level. + ledger.NewAccount(appAddr(888), 12*MakeTestProto().MinTxnFee) + TestApp(t, create, ep) + TestApp(t, freeze, ep, "unavailable Asset 5000") tx.ForeignAssets = []basics.AssetIndex{basics.AssetIndex(5000)} tx.ApplicationArgs = [][]byte{{0x01}} @@ -805,6 +831,10 @@ func TestAssetFreeze(t *testing.T) { holding, err = ledger.AssetHolding(tx.Receiver, 5000) require.NoError(t, err) require.Equal(t, false, holding.Frozen) + + // Malformed + TestApp(t, missingFreezeAccount, ep, "freeze account cannot be empty") + TestApp(t, missingAssetID, ep, "asset ID cannot be zero") }) } diff --git a/data/transactions/logic/ledger_test.go b/data/transactions/logic/ledger_test.go index eb55406075..fb95d70d73 100644 --- a/data/transactions/logic/ledger_test.go +++ b/data/transactions/logic/ledger_test.go @@ -731,17 +731,20 @@ func (l *Ledger) axfer(from basics.Address, xfer transactions.AssetTransferTxnFi } fholding, ok := fbr.holdings[aid] if !ok { - if from == to && amount == 0 { - // opt in - if params, exists := l.assets[aid]; exists { - fbr.holdings[aid] = basics.AssetHolding{ - Frozen: params.DefaultFrozen, + if amount == 0 { + if from == to { + // opt in + if params, exists := l.assets[aid]; exists { + fbr.holdings[aid] = basics.AssetHolding{ + Frozen: params.DefaultFrozen, + } + } else { + return fmt.Errorf("Asset (%d) does not exist", aid) } - return nil } - return fmt.Errorf("Asset (%d) does not exist", aid) + } else { + return fmt.Errorf("Sender (%s) not opted in to %d", from, aid) } - return fmt.Errorf("Sender (%s) not opted in to %d", from, aid) } if fholding.Frozen { return fmt.Errorf("Sender (%s) is frozen for %d", from, aid) diff --git a/data/transactions/transaction.go b/data/transactions/transaction.go index 70b2068f3c..9ced317c04 100644 --- a/data/transactions/transaction.go +++ b/data/transactions/transaction.go @@ -337,11 +337,36 @@ func (tx Transaction) WellFormed(spec SpecialAddresses, proto config.ConsensusPa return err } - case protocol.AssetConfigTx, protocol.AssetTransferTx, protocol.AssetFreezeTx: + case protocol.AssetConfigTx: if !proto.Asset { return fmt.Errorf("asset transaction not supported") } + err := tx.AssetConfigTxnFields.wellFormed(proto) + if err != nil { + return err + } + + case protocol.AssetTransferTx: + if !proto.Asset { + return fmt.Errorf("asset transaction not supported") + } + + err := tx.AssetTransferTxnFields.wellFormed() + if err != nil { + return err + } + + case protocol.AssetFreezeTx: + if !proto.Asset { + return fmt.Errorf("asset transaction not supported") + } + + err := tx.AssetFreezeTxnFields.wellFormed() + if err != nil { + return err + } + case protocol.ApplicationCallTx: if !proto.Application { return fmt.Errorf("application transaction not supported") @@ -431,18 +456,6 @@ func (tx Transaction) WellFormed(spec SpecialAddresses, proto config.ConsensusPa if len(tx.Note) > proto.MaxTxnNoteBytes { return fmt.Errorf("transaction note too big: %d > %d", len(tx.Note), proto.MaxTxnNoteBytes) } - if len(tx.AssetConfigTxnFields.AssetParams.AssetName) > proto.MaxAssetNameBytes { - return fmt.Errorf("transaction asset name too big: %d > %d", len(tx.AssetConfigTxnFields.AssetParams.AssetName), proto.MaxAssetNameBytes) - } - if len(tx.AssetConfigTxnFields.AssetParams.UnitName) > proto.MaxAssetUnitNameBytes { - return fmt.Errorf("transaction asset unit name too big: %d > %d", len(tx.AssetConfigTxnFields.AssetParams.UnitName), proto.MaxAssetUnitNameBytes) - } - if len(tx.AssetConfigTxnFields.AssetParams.URL) > proto.MaxAssetURLBytes { - return fmt.Errorf("transaction asset url too big: %d > %d", len(tx.AssetConfigTxnFields.AssetParams.URL), proto.MaxAssetURLBytes) - } - if tx.AssetConfigTxnFields.AssetParams.Decimals > proto.MaxAssetDecimals { - return fmt.Errorf("transaction asset decimals is too high (max is %d)", proto.MaxAssetDecimals) - } if tx.Sender == spec.RewardsPool { // this check is just to be safe, but reaching here seems impossible, since it requires computing a preimage of rwpool return fmt.Errorf("transaction from incentive pool is invalid")