services/horizon: Improve transaction precondition omitempty behavi…

…or (#4360)

* Omit max_ledger when it's 0
* Drops minSeqAge from response when it's 0:

Because we use a string for the field (to work around languages
that don't natively support uint64 values) in the JSON response,
but the field is a non-pointer in the XDR, it will always be set
and will be 0 by default.

Since it's a duration, 0 is effectively "not set", and so it
should be omitted from the resulting precondition.

Caveat: if someone explicitly sets it to 0 when building their tx,
this is equivalent to not setting it, so they may be confused
about its omission, but this is working as intended.

protocols/horizon/main.go

@@ -552,7 +552,7 @@ type TransactionPreconditionsTimebounds struct {
 
 type TransactionPreconditionsLedgerbounds struct {
 	MinLedger uint32 `json:"min_ledger"`
-	MaxLedger uint32 `json:"max_ledger"`
+	MaxLedger uint32 `json:"max_ledger,omitempty"`
 }
 
 // FeeBumpTransaction contains information about a fee bump transaction

services/horizon/internal/db2/history/main.go

@@ -720,7 +720,8 @@ func (t *Transaction) HasPreconditions() bool {
 	return !t.TimeBounds.Null ||
 		!t.LedgerBounds.Null ||
 		t.MinAccountSequence.Valid ||
-		t.MinAccountSequenceAge.Valid ||
+		(t.MinAccountSequenceAge.Valid &&
+			t.MinAccountSequenceAge.String != "0") ||
 		t.MinAccountSequenceLedgerGap.Valid ||
 		len(t.ExtraSigners) > 0
 }

services/horizon/internal/db2/history/transaction_ledger_bounds.go

@@ -79,6 +79,9 @@ func formatLedgerBounds(ledgerBounds *xdr.LedgerBounds) LedgerBounds {
 
 	return LedgerBounds{
 		MinLedger: null.IntFrom(int64(ledgerBounds.MinLedger)),
-		MaxLedger: null.IntFrom(int64(ledgerBounds.MaxLedger)),
+		// elide max_ledger if it's 0 since that means no upper bound
+		MaxLedger: null.NewInt(
+			int64(ledgerBounds.MaxLedger),
+			ledgerBounds.MaxLedger > 0),
 	}
 }

services/horizon/internal/resourceadapter/transaction.go

@@ -84,7 +84,7 @@ func PopulateTransaction(
 		dest.Preconditions.MinAccountSequence = fmt.Sprint(row.MinAccountSequence.Int64)
 	}
 
-	if row.MinAccountSequenceAge.Valid {
+	if row.MinAccountSequenceAge.Valid && row.MinAccountSequenceAge.String != "0" {
 		dest.Preconditions.MinAccountSequenceAge = row.MinAccountSequenceAge.String
 	}
 

services/horizon/internal/resourceadapter/transaction_test.go

@@ -2,6 +2,7 @@ package resourceadapter
 
 import (
 	"encoding/base64"
+	"encoding/json"
 	"fmt"
 	"testing"
 	"time"
@@ -293,6 +294,69 @@ func TestPopulateTransaction_PreconditionsV2(t *testing.T) {
 	}
 }
 
+// TestPopulateTransaction_PreconditionsV2_Omissions ensures that all fields are
+// omitted from the final JSON when appropriate.
+func TestPopulateTransaction_PreconditionsV2_Omissions(t *testing.T) {
+	ctx, _ := test.ContextWithLogBuffer()
+	tt := assert.New(t)
+
+	jsonifyTx := func(tx history.TransactionWithoutLedger) (
+		Transaction, map[string]interface{},
+	) {
+		var dest Transaction
+		generic := map[string]interface{}{}
+
+		row := history.Transaction{TransactionWithoutLedger: tx}
+		tt.NoError(PopulateTransaction(ctx, row.TransactionHash, &dest, row))
+
+		bytes, err := dest.MarshalJSON()
+		tt.NoError(err)
+		tt.NoError(json.Unmarshal(bytes, &generic)) // round-trip
+
+		return dest, generic
+	}
+
+	for _, tx := range []history.TransactionWithoutLedger{
+		{
+			// minimum precondition so that the field exists in general
+			MinAccountSequenceLedgerGap: null.IntFrom(0),
+			TimeBounds:                  history.TimeBounds{Null: true},
+			LedgerBounds:                history.LedgerBounds{Null: true},
+			MinAccountSequence:          null.IntFromPtr(nil),
+			MinAccountSequenceAge:       null.StringFrom("0"),
+			ExtraSigners:                pq.StringArray{},
+		}, {
+			MinAccountSequenceLedgerGap: null.IntFrom(0),
+			TimeBounds:                  history.TimeBounds{Null: true},
+			LedgerBounds:                history.LedgerBounds{Null: true},
+			MinAccountSequence:          null.IntFromPtr(nil),
+			MinAccountSequenceAge:       null.StringFromPtr(nil),
+			ExtraSigners:                nil,
+		},
+	} {
+		dest, js := jsonifyTx(tx)
+		if !tt.Contains(js, "preconditions") ||
+			!tt.IsType(map[string]interface{}{}, js["preconditions"]) {
+			tt.FailNow("expected 'preconditions' object in transaction")
+		}
+
+		precond := js["preconditions"].(map[string]interface{})
+		helpStr := fmt.Sprintf("input precondition: %+v", dest.Preconditions)
+		tt.NotContainsf(precond, "timebounds", helpStr)
+		tt.NotContainsf(precond, "ledgerbounds", helpStr)
+		tt.NotContainsf(precond, "min_account_sequence", helpStr)
+		tt.NotContainsf(precond, "min_account_sequence_age", helpStr)
+		tt.NotContainsf(precond, "min_account_sequence_ledger_gap", helpStr)
+		tt.NotContainsf(precond, "extra_signers", helpStr)
+
+		// If we drop the minimum precondition, the structure should cease to
+		// exist entirely.
+		tx.MinAccountSequenceLedgerGap = null.IntFromPtr(nil)
+		dest, js = jsonifyTx(tx)
+		tt.NotContains(js, "preconditions")
+	}
+}
+
 func TestFeeBumpTransaction(t *testing.T) {
 	ctx, _ := test.ContextWithLogBuffer()
 	dest := Transaction{}