Preprocess _muxed_id fields before unmarshalling from the DB (#3722)

protocols/horizon/operations/main.go

@@ -88,7 +88,7 @@ type CreateAccount struct {
 	StartingBalance string `json:"starting_balance"`
 	Funder          string `json:"funder"`
 	FunderMuxed     string `json:"funder_muxed,omitempty"`
-	FunderMuxedID   uint64 `json:"funder_muxed_id,omitempty"`
+	FunderMuxedID   uint64 `json:"funder_muxed_id,omitempty,string"`
 	Account         string `json:"account"`
 }
 
@@ -99,10 +99,10 @@ type Payment struct {
 	base.Asset
 	From        string `json:"from"`
 	FromMuxed   string `json:"from_muxed,omitempty"`
-	FromMuxedID uint64 `json:"from_muxed_id,omitempty"`
+	FromMuxedID uint64 `json:"from_muxed_id,omitempty,string"`
 	To          string `json:"to"`
 	ToMuxed     string `json:"to_muxed,omitempty"`
-	ToMuxedID   uint64 `json:"to_muxed_id,omitempty"`
+	ToMuxedID   uint64 `json:"to_muxed_id,omitempty,string"`
 	Amount      string `json:"amount"`
 }
 

services/horizon/CHANGELOG.md

@@ -6,6 +6,12 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+## v2.5.2
+
+**Upgrading to this version from <= v2.1.1 will trigger a state rebuild. During this process (which can take up to 20 minutes), Horizon will not ingest new ledgers.**
+
+* Fix a bug in the method unmarshaling payment operation details. ([#3722](https://github.com/stellar/go/pull/3722))
+
 ## v2.5.1
 
 **Upgrading to this version from <= v2.1.1 will trigger a state rebuild. During this process (which can take up to 20 minutes), Horizon will not ingest new ledgers.**

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

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"strings"
 	"text/template"
 
 	sq "github.com/Masterminds/squirrel"
@@ -24,13 +25,37 @@ func (r *Operation) UnmarshalDetails(dest interface{}) error {
 	if !r.DetailsString.Valid {
 		return nil
 	}
-
-	err := json.Unmarshal([]byte(r.DetailsString.String), &dest)
+	preprocessedDetails, err := preprocessDetails(r.DetailsString.String)
+	if err != nil {
+		return errors.Wrap(err, "error in unmarshal")
+	}
+	err = json.Unmarshal(preprocessedDetails, &dest)
 	if err != nil {
-		err = errors.Wrap(err, "error in unmarshal")
+		return errors.Wrap(err, "error in unmarshal")
 	}
 
-	return err
+	return nil
+}
+
+func preprocessDetails(details string) ([]byte, error) {
+	var dest map[string]interface{}
+	// Create a decoder using Number instead of float64 when decoding
+	// (so that decoding covers the full uint64 range)
+	decoder := json.NewDecoder(strings.NewReader(details))
+	decoder.UseNumber()
+	if err := decoder.Decode(&dest); err != nil {
+		return nil, err
+	}
+	for k, v := range dest {
+		if strings.HasSuffix(k, "_muxed_id") {
+			if vNumber, ok := v.(json.Number); ok {
+				// transform it into a string so that _muxed_id unmarshalling works with `,string` tags
+				// see https://github.com/stellar/go/pull/3716#issuecomment-867057436
+				dest[k] = vNumber.String()
+			}
+		}
+	}
+	return json.Marshal(dest)
 }
 
 var feeStatsQueryTemplate = template.Must(template.New("trade_aggregations_query").Parse(`

services/horizon/internal/ingest/processors/operations_processor.go

@@ -216,6 +216,10 @@ func addAccountAndMuxedAccountDetails(result map[string]interface{}, a xdr.Muxed
 	result[prefix] = accid.Address()
 	if a.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 {
 		result[prefix+"_muxed"] = a.Address()
+		// _muxed_id fields should had ideally been stored in the DB as a string instead of uint64
+		// due to Javascript not being able to handle them, see https://github.com/stellar/go/issues/3714
+		// However, we released this code in the wild before correcting it. Thus, what we do is
+		// work around it (by preprocessing it into a string) in Operation.UnmarshalDetails()
 		result[prefix+"_muxed_id"] = uint64(a.Med25519.Id)
 	}
 }

services/horizon/internal/integration/muxed_account_details_test.go

@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"math"
 	"testing"
 
 	"github.com/stellar/go/clients/horizonclient"
@@ -27,15 +28,16 @@ func TestMuxedAccountDetails(t *testing.T) {
 	source := xdr.MuxedAccount{
 		Type: xdr.CryptoKeyTypeKeyTypeMuxedEd25519,
 		Med25519: &xdr.MuxedAccountMed25519{
-			Id:      0xcafebabe,
+			Id:      0xcafebabecafebabe,
 			Ed25519: *masterAcID.Ed25519,
 		},
 	}
 
 	destination := xdr.MuxedAccount{
 		Type: xdr.CryptoKeyTypeKeyTypeMuxedEd25519,
 		Med25519: &xdr.MuxedAccountMed25519{
-			Id:      0xdeadbeef,
+			// Make sure we cover the full uint64 range
+			Id:      math.MaxUint64,
 			Ed25519: *destinationAcID.Ed25519,
 		},
 	}

services/horizon/internal/integration/muxed_operations_test.go

@@ -0,0 +1,134 @@
+package integration
+
+import (
+	"testing"
+
+	"github.com/stellar/go/clients/horizonclient"
+	"github.com/stellar/go/keypair"
+	"github.com/stellar/go/protocols/horizon/effects"
+	"github.com/stellar/go/services/horizon/internal/test/integration"
+	"github.com/stellar/go/txnbuild"
+	"github.com/stellar/go/xdr"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMuxedOperations(t *testing.T) {
+	itest := integration.NewTest(t, integration.Config{ProtocolVersion: 17})
+
+	sponsored := keypair.MustRandom()
+	// Is there an easier way?
+	sponsoredMuxed := xdr.MustMuxedAddress(sponsored.Address())
+	sponsoredMuxed.Type = xdr.CryptoKeyTypeKeyTypeMuxedEd25519
+	sponsoredMuxed.Med25519 = &xdr.MuxedAccountMed25519{
+		Ed25519: *sponsoredMuxed.Ed25519,
+		Id:      100,
+	}
+
+	master := itest.Master()
+	masterMuxed := xdr.MustMuxedAddress(master.Address())
+	masterMuxed.Type = xdr.CryptoKeyTypeKeyTypeMuxedEd25519
+	masterMuxed.Med25519 = &xdr.MuxedAccountMed25519{
+		Ed25519: *masterMuxed.Ed25519,
+		Id:      200,
+	}
+
+	ops := []txnbuild.Operation{
+		&txnbuild.BeginSponsoringFutureReserves{
+			SponsoredID: sponsored.Address(),
+		},
+		&txnbuild.CreateAccount{
+			Destination: sponsored.Address(),
+			Amount:      "100",
+		},
+		&txnbuild.ChangeTrust{
+			SourceAccount: sponsoredMuxed.Address(),
+			Line:          txnbuild.CreditAsset{"ABCD", master.Address()},
+			Limit:         txnbuild.MaxTrustlineLimit,
+		},
+		&txnbuild.ManageSellOffer{
+			SourceAccount: sponsoredMuxed.Address(),
+			Selling:       txnbuild.NativeAsset{},
+			Buying:        txnbuild.CreditAsset{"ABCD", master.Address()},
+			Amount:        "3",
+			Price:         "1",
+		},
+		// This will generate a trade effect:
+		&txnbuild.ManageSellOffer{
+			SourceAccount: masterMuxed.Address(),
+			Selling:       txnbuild.CreditAsset{"ABCD", master.Address()},
+			Buying:        txnbuild.NativeAsset{},
+			Amount:        "3",
+			Price:         "1",
+		},
+		&txnbuild.ManageData{
+			SourceAccount: sponsoredMuxed.Address(),
+			Name:          "test",
+			Value:         []byte("test"),
+		},
+		&txnbuild.Payment{
+			SourceAccount: sponsoredMuxed.Address(),
+			Destination:   master.Address(),
+			Amount:        "1",
+			Asset:         txnbuild.NativeAsset{},
+		},
+		&txnbuild.CreateClaimableBalance{
+			SourceAccount: sponsoredMuxed.Address(),
+			Amount:        "2",
+			Asset:         txnbuild.NativeAsset{},
+			Destinations: []txnbuild.Claimant{
+				txnbuild.NewClaimant(keypair.MustRandom().Address(), nil),
+			},
+		},
+		&txnbuild.EndSponsoringFutureReserves{
+			SourceAccount: sponsored.Address(),
+		},
+	}
+	txResp, err := itest.SubmitMultiSigOperations(itest.MasterAccount(), []*keypair.Full{master, sponsored}, ops...)
+	assert.NoError(t, err)
+	assert.True(t, txResp.Successful)
+
+	ops = []txnbuild.Operation{
+		// Remove subentries to be able to merge account
+		&txnbuild.Payment{
+			SourceAccount: sponsoredMuxed.Address(),
+			Destination:   master.Address(),
+			Amount:        "3",
+			Asset:         txnbuild.CreditAsset{"ABCD", master.Address()},
+		},
+		&txnbuild.ChangeTrust{
+			SourceAccount: sponsoredMuxed.Address(),
+			Line:          txnbuild.CreditAsset{"ABCD", master.Address()},
+			Limit:         "0",
+		},
+		&txnbuild.ManageData{
+			SourceAccount: sponsoredMuxed.Address(),
+			Name:          "test",
+		},
+		&txnbuild.AccountMerge{
+			SourceAccount: sponsoredMuxed.Address(),
+			Destination:   masterMuxed.Address(),
+		},
+	}
+	txResp, err = itest.SubmitMultiSigOperations(itest.MasterAccount(), []*keypair.Full{master, sponsored}, ops...)
+	assert.NoError(t, err)
+	assert.True(t, txResp.Successful)
+
+	// Check if no 5xx after processing the tx above
+	// TODO expand it to test actual muxed fields
+	_, err = itest.Client().Operations(horizonclient.OperationRequest{Limit: 200})
+	assert.NoError(t, err, "/operations failed")
+
+	_, err = itest.Client().Payments(horizonclient.OperationRequest{Limit: 200})
+	assert.NoError(t, err, "/payments failed")
+
+	effectsPage, err := itest.Client().Effects(horizonclient.EffectRequest{Limit: 200})
+	assert.NoError(t, err, "/effects failed")
+
+	for _, effect := range effectsPage.Embedded.Records {
+		if effect.GetType() == "trade" {
+			trade := effect.(effects.Trade)
+			oneSet := trade.AccountMuxedID != 0 || trade.SellerMuxedID != 0
+			assert.True(t, oneSet, "at least one of account_muxed_id, seller_muxed_id must be set")
+		}
+	}
+}

txnbuild/account_merge.go

@@ -63,9 +63,9 @@ func (am *AccountMerge) FromXDR(xdrOp xdr.Operation, withMuxedAccounts bool) err
 func (am *AccountMerge) Validate(withMuxedAccounts bool) error {
 	var err error
 	if withMuxedAccounts {
-		_, err = xdr.AddressToAccountId(am.Destination)
-	} else {
 		_, err = xdr.AddressToMuxedAccount(am.Destination)
+	} else {
+		_, err = xdr.AddressToAccountId(am.Destination)
 	}
 	if err != nil {
 		return NewValidationError("Destination", err.Error())

txnbuild/account_merge_test.go

@@ -23,7 +23,7 @@ func TestAccountMergeValidate(t *testing.T) {
 		},
 	)
 	if assert.Error(t, err) {
-		expected := "invalid address"
+		expected := "minimum valid length is 5"
 		assert.Contains(t, err.Error(), expected)
 	}
 }