pool fees in a txgroup (#2173)

A fair amount of complexity is introduced in transaction groups when the goal is to let some entity perform an action at the expense of another. For example, a contract account might be willing to perform an exchange, but expects the caller to compensate it to replace the fee that the contract account must pay.

This changes fee accounting to simplify these situations. Rather than check that each txn in a txn group meets the min fee, the txgroup is checked as a whole to ensure the total fee exceeds n*min_fee for an n member txgroup.

Author: jannotti

cmd/goal/clerk.go

@@ -375,6 +375,14 @@ var sendCmd = &cobra.Command{
 			payment.RekeyTo = rekeyTo
 		}
 
+		// ConstructPayment fills in the suggested fee when fee=0. But if the user actually used --fee=0 on the
+		// commandline, we ought to do what they asked (especially now that zero or low fees make sense in
+		// combination with other txns that cover the groups's fee.
+		explicitFee := cmd.Flags().Changed("fee")
+		if explicitFee {
+			payment.Fee = basics.MicroAlgos{Raw: fee}
+		}
+
 		var stx transactions.SignedTxn
 		if lsig.Logic != nil {
 

config/consensus.go

@@ -103,6 +103,11 @@ type ConsensusParams struct {
 	// a way of making the spender subsidize the cost of storing this transaction.
 	MinTxnFee uint64
 
+	// EnableFeePooling specifies that the sum of the fees in a
+	// group must exceed one MinTxnFee per Txn, rather than check that
+	// each Txn has a MinFee.
+	EnableFeePooling bool
+
 	// RewardUnit specifies the number of MicroAlgos corresponding to one reward
 	// unit.
 	//
@@ -944,6 +949,8 @@ func initConsensusProtocols() {
 	// Increase asset URL length to allow for IPFS URLs
 	vFuture.MaxAssetURLBytes = 96
 
+	vFuture.EnableFeePooling = true
+
 	Consensus[protocol.ConsensusFuture] = vFuture
 }
 

data/transactions/transaction.go

@@ -472,7 +472,7 @@ func (tx Transaction) WellFormed(spec SpecialAddresses, proto config.ConsensusPa
 		}
 	}
 
-	if tx.Fee.LessThan(basics.MicroAlgos{Raw: proto.MinTxnFee}) {
+	if !proto.EnableFeePooling && tx.Fee.LessThan(basics.MicroAlgos{Raw: proto.MinTxnFee}) {
 		if tx.Type == protocol.CompactCertTx {
 			// Zero fee allowed for compact cert txn.
 		} else {

data/transactions/verify/txn.go

@@ -108,10 +108,6 @@ func Txn(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext) error {
 		return err
 	}
 
-	if s.Txn.Src().IsZero() {
-		return errors.New("empty address")
-	}
-
 	return stxnVerifyCore(s, txnIdx, groupCtx)
 }
 
@@ -121,13 +117,34 @@ func TxnGroup(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader,
 	if err != nil {
 		return nil, err
 	}
+
+	minFeeCount := uint64(0)
+	feesPaid := uint64(0)
 	for i, stxn := range stxs {
 		err = Txn(&stxn, i, groupCtx)
 		if err != nil {
 			err = fmt.Errorf("transaction %+v invalid : %w", stxn, err)
 			return
 		}
+		if stxn.Txn.Type != protocol.CompactCertTx {
+			minFeeCount++
+		}
+		feesPaid = basics.AddSaturate(feesPaid, stxn.Txn.Fee.Raw)
 	}
+	feeNeeded, overflow := basics.OMul(groupCtx.consensusParams.MinTxnFee, minFeeCount)
+	if overflow {
+		err = fmt.Errorf("txgroup fee requirement overflow")
+		return
+	}
+	// feesPaid may have saturated. That's ok. Since we know
+	// feeNeeded did not overlfow, simple comparision tells us
+	// feesPaid was enough.
+	if feesPaid < feeNeeded {
+		err = fmt.Errorf("txgroup had %d in fees, which is less than the minimum %d * %d",
+			feesPaid, minFeeCount, groupCtx.consensusParams.MinTxnFee)
+		return
+	}
+
 	if cache != nil {
 		cache.Add(stxs, groupCtx)
 	}

ledger/apply/payment_test.go

@@ -221,6 +221,11 @@ func TestPaymentValidation(t *testing.T) {
 		if badFee.WellFormed(spec, tc.Proto) == nil {
 			t.Errorf("transaction with no fee %#v verified incorrectly", badFee)
 		}
+		badFee.Fee.Raw = 1
+		if badFee.WellFormed(spec, tc.Proto) == nil {
+			t.Errorf("transaction with low fee %#v verified incorrectly", badFee)
+		}
+
 	}
 }
 

test/e2e-go/cli/goal/clerk_test.go

@@ -44,14 +44,14 @@ func TestClerkSendNoteEncoding(t *testing.T) {
 	account := accounts[0].Address
 
 	const noteText = "Sample Text-based Note"
-	txID, err := fixture.ClerkSend(account, account, 100, 1, noteText)
+	txID, err := fixture.ClerkSend(account, account, 100, 1000, noteText)
 	a.NoError(err)
 	a.NotEmpty(txID)
 
 	// Send 2nd txn using the note encoded as base-64 (using --noteb64)
 	originalNoteb64Text := "Noteb64-encoded text With Binary \u0001x1x0x3"
 	noteb64 := base64.StdEncoding.EncodeToString([]byte(originalNoteb64Text))
-	txID2, err := fixture.ClerkSendNoteb64(account, account, 100, 1, noteb64)
+	txID2, err := fixture.ClerkSendNoteb64(account, account, 100, 1000, noteb64)
 	a.NoError(err)
 	a.NotEmpty(txID2)
 

test/scripts/e2e_subs/single-payer-swap.sh

@@ -0,0 +1,57 @@
+#!/bin/bash
+
+filename=$(basename "$0")
+scriptname="${filename%.*}"
+date "+${scriptname} start %Y%m%d_%H%M%S"
+
+set -e
+set -x
+set -o pipefail
+export SHELLOPTS
+
+WALLET=$1
+
+gcmd="goal -w ${WALLET}"
+
+PAYER=$(${gcmd} account list|awk '{ print $3 }')
+MOOCHER=$(${gcmd} account new|awk '{ print $6 }')
+
+# Fund MOOCHER, 1M
+${gcmd} clerk send -a 1000000 -f "${PAYER}" -t "${MOOCHER}"
+
+# Payer and Moocher are going to pay each other 100 mAlgos, but
+# Moocher is not going to pay the minfee (Payer will pay double)
+
+# Fair number of temporary files, just cd into TEMPDIR first
+cd ${TEMPDIR}
+
+# Check a low fee from moocher
+${gcmd} clerk send -a 100 -f "${MOOCHER}" -t "${PAYER}" --fee 2 -o cheap.txn
+# Since goal was modified to allow < minfee when this feature was added, let's confirm
+msgpacktool -d < cheap.txn | grep fee | grep 2
+${gcmd} clerk send -a 100 -f "${PAYER}" -t "${MOOCHER}" --fee 2000 -o expensive.txn
+cat cheap.txn expensive.txn > both.txn
+${gcmd} clerk group -i both.txn -o group.txn
+${gcmd} clerk sign -i group.txn -o group.stx
+${gcmd} clerk rawsend -f group.stx
+
+# Check a zero fee from moocher
+${gcmd} clerk send -a 100 -f "${MOOCHER}" -t "${PAYER}" --fee 0 -o cheap.txn
+# Since goal was modified to allow zero when this feature was added, let's confirm
+# that it's not encoded (should be "omitempty")
+set +e
+FOUND=$(msgpacktool -d < cheap.txn | grep fee)
+set -e
+if [[ $FOUND != "" ]]; then
+    date "+{scriptname} FAIL fee was improperly encoded $FOUND %Y%m%d_%H%M%S"
+    false
+fi
+
+${gcmd} clerk send -a 100 -f "${PAYER}" -t "${MOOCHER}" --fee 2000 -o expensive.txn
+cat cheap.txn expensive.txn > both.txn
+${gcmd} clerk group -i both.txn -o group.txn
+${gcmd} clerk sign -i group.txn -o group.stx
+${gcmd} clerk rawsend -f group.stx
+
+
+date "+${scriptname} OK %Y%m%d_%H%M%S"