diff --git a/CHANGELOG.md b/CHANGELOG.md index 88f4e39fb8..d741eeaa2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,55 @@ This monorepo contains a number of sdk's: Official project releases may be found here: https://github.com/stellar/go-stellar-sdk/releases ## Pending +## [0.6.1] - 2026-08-10 + +Backport release for Horizon 27.0.1, cut from `v0.6.0`. + +### Bug Fixes +* xdr: `Asset.LessThan` now orders assets the way the protocol does — by the raw 32-byte issuer key — instead of by base32 strkey text, and `xdr.NewPoolId` requires strictly `a < b`, rejecting reversed and identical pairs ([#5974](https://github.com/stellar/go-stellar-sdk/pull/5974)) +* txnbuild: liquidity pool operations reject asset pairs that are not strictly ordered — previously these built transactions that stellar-core rejects; see the [txnbuild changelog](./txnbuild/CHANGELOG.md) ([#5974](https://github.com/stellar/go-stellar-sdk/pull/5974)) +* processors/token_transfer: trustline revocation now compares liquidity pool assets by value instead of pointer identity, fixing wrong-leg selection when burning pool shares ([#5974](https://github.com/stellar/go-stellar-sdk/pull/5974)) +* clients/stellartoml: `GetStellarToml` now validates the domain before issuing the request, matching `GetStellarTomlByAddress` ([#5970](https://github.com/stellar/go-stellar-sdk/pull/5970)) + +### Updates +* ingest/ledgerbackend: Updated the embedded `captive-core-pubnet.cfg`, replacing SatoshiPay's validators with Obsrvr's ([#5963](https://github.com/stellar/go-stellar-sdk/pull/5963)) +* go.mod: Bumped github.com/stellar/go-xdr to dc590f1 ([#5974](https://github.com/stellar/go-stellar-sdk/pull/5974)) + +## [0.6.0] - 2026-06-09 + +Adds support for Protocol 27 (CAP-0071). + +### New Features +* xdr: Protocol 27 (CAP-0071) XDR ([#5945](https://github.com/stellar/go-stellar-sdk/pull/5945), [#5947](https://github.com/stellar/go-stellar-sdk/pull/5947)) +* xdr: Added zero-copy XDR view types and code generator ([#5937](https://github.com/stellar/go-stellar-sdk/pull/5937)) +* ingest/ledgerbackend: Integrated XDR views into the buffered storage backend and added `GetLedgerRaw` ([#5941](https://github.com/stellar/go-stellar-sdk/pull/5941)) +* ingest/ledgerbackend: Added `LedgerStream` streaming ingestion API ([#5944](https://github.com/stellar/go-stellar-sdk/pull/5944)) +* ingest/loadtest: Added stellar-core apply-load tooling ([#5940](https://github.com/stellar/go-stellar-sdk/pull/5940)) +* apiclient: Support non-JSON responses via `ResponseType` ([#5939](https://github.com/stellar/go-stellar-sdk/pull/5939)) + +### Bug Fixes +* services/stellar-archivist: Fixed nil pointer panics in `S3Storage.ListFiles` ([#5934](https://github.com/stellar/go-stellar-sdk/pull/5934)) +* strkey: Bounded decode input length to avoid unnecessary allocation ([#5935](https://github.com/stellar/go-stellar-sdk/pull/5935)) +* txnbuild: Validate payload length in contract address decoding ([#5943](https://github.com/stellar/go-stellar-sdk/pull/5943)) + +### Updates +* go.mod: Bumped github.com/stellar/go-xdr to a87d4d0 ([#5938](https://github.com/stellar/go-stellar-sdk/pull/5938)) + +## [0.5.0] - 2026-04-07 + +### Bug Fixes +* ingest: Fixed `VerifyEvents` to handle amounts exceeding the int64 range ([#5932](https://github.com/stellar/go-stellar-sdk/pull/5932)) + +## [0.4.0] - 2026-04-01 + +Adds support for Protocol 26. + +### New Features +* xdr: Protocol 26 support, merged from protocol-next ([#5930](https://github.com/stellar/go-stellar-sdk/pull/5930)) + +### Bug Fixes +* support/datastore: Fixed `ListFilePath` for a datastore bucket with no prefix ([#5923](https://github.com/stellar/go-stellar-sdk/pull/5923)) + ## [0.3.0] ### Security Fixes diff --git a/clients/stellartoml/client.go b/clients/stellartoml/client.go index d4e23901ae..b5183dc089 100644 --- a/clients/stellartoml/client.go +++ b/clients/stellartoml/client.go @@ -6,12 +6,21 @@ import ( "net/http" "github.com/BurntSushi/toml" + "github.com/asaskevich/govalidator" "github.com/stellar/go-stellar-sdk/address" "github.com/stellar/go-stellar-sdk/support/errors" ) // GetStellarToml returns stellar.toml file for a given domain func (c *Client) GetStellarToml(domain string) (resp *Response, err error) { + // GetStellarTomlByAddress already rejects a non-DNS domain (via address.Split), + // but this entry point historically did not. Apply the same check here so both + // entry points behave the same way and neither will build a request URL out of + // an IP literal, a "user@host" string, or anything else that isn't a hostname. + if !govalidator.IsDNSName(domain) { + return nil, errors.Errorf("invalid domain: %q is not a valid DNS name", domain) + } + var hresp *http.Response hresp, err = c.HTTP.Get(c.url(domain)) if err != nil { diff --git a/clients/stellartoml/client_test.go b/clients/stellartoml/client_test.go index 212cdbf207..21d5d81f36 100644 --- a/clients/stellartoml/client_test.go +++ b/clients/stellartoml/client_test.go @@ -22,6 +22,24 @@ func TestClientURL(t *testing.T) { assert.Equal(t, "http://stellar.org/.well-known/stellar.toml", c.url("stellar.org")) } +func TestClientRejectsNonDNSDomain(t *testing.T) { + h := httptest.NewClient() + c := &Client{HTTP: h} + + for _, domain := range []string{ + "127.0.0.1", + "127.0.0.1:8080", + "user@stellar.org", + "169.254.169.254", + "stellar.org/../../evil", + } { + _, err := c.GetStellarToml(domain) + if assert.Error(t, err, "domain %q should be rejected", domain) { + assert.Contains(t, err.Error(), "not a valid DNS name") + } + } +} + func TestClient(t *testing.T) { h := httptest.NewClient() c := &Client{HTTP: h} diff --git a/go.mod b/go.mod index f57e0057e3..5bf5e6a2df 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,7 @@ require ( github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.17.0 - github.com/stellar/go-xdr v0.0.0-20260529210834-0bf8f4956364 + github.com/stellar/go-xdr v0.0.0-20260806060815-dc590f17552a github.com/stretchr/testify v1.10.0 github.com/tyler-smith/go-bip39 v0.0.0-20180618194314-52158e4697b8 github.com/xdrpp/goxdr v0.1.1 diff --git a/go.sum b/go.sum index aaa794932a..6de6017c13 100644 --- a/go.sum +++ b/go.sum @@ -453,8 +453,8 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.17.0 h1:I5txKw7MJasPL/BrfkbA0Jyo/oELqVmux4pR/UxOMfI= github.com/spf13/viper v1.17.0/go.mod h1:BmMMMLQXSbcHK6KAOiFLz0l5JHrU89OdIRHvsk0+yVI= -github.com/stellar/go-xdr v0.0.0-20260529210834-0bf8f4956364 h1:gOKrfuWdZ92LFlv0TAwgZ7OsWKeBsOMDlGLyFgduI1w= -github.com/stellar/go-xdr v0.0.0-20260529210834-0bf8f4956364/go.mod h1:If+U9Z1W5xU97VrOgJandQT+2dN7/iOpkCrxBJEyF80= +github.com/stellar/go-xdr v0.0.0-20260806060815-dc590f17552a h1:40YIhQusSioBKDW5Tr+YdjoXphcVeu7eXUS2ibMQBTs= +github.com/stellar/go-xdr v0.0.0-20260806060815-dc590f17552a/go.mod h1:If+U9Z1W5xU97VrOgJandQT+2dN7/iOpkCrxBJEyF80= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= diff --git a/ingest/ledgerbackend/configs/captive-core-pubnet.cfg b/ingest/ledgerbackend/configs/captive-core-pubnet.cfg index 6ea829fe2d..b2a2acc411 100644 --- a/ingest/ledgerbackend/configs/captive-core-pubnet.cfg +++ b/ingest/ledgerbackend/configs/captive-core-pubnet.cfg @@ -18,7 +18,7 @@ HOME_DOMAIN="www.franklintempleton.com" QUALITY="HIGH" [[HOME_DOMAINS]] -HOME_DOMAIN="satoshipay" +HOME_DOMAIN="stellar.withobsrvr.com" QUALITY="HIGH" [[HOME_DOMAINS]] @@ -97,25 +97,25 @@ HISTORY="curl -sf https://stellar-history-usw.franklintempleton.com/azuswshf401/ HOME_DOMAIN="www.franklintempleton.com" [[VALIDATORS]] -NAME="satoshipay_de" -PUBLIC_KEY="GC5SXLNAM3C4NMGK2PXK4R34B5GNZ47FYQ24ZIBFDFOCU6D4KBN4POAE" -ADDRESS="stellar-de-fra.satoshipay.io:11625" -HISTORY="curl -sf https://stellar-history-de-fra.satoshipay.io/{0} -o {1}" -HOME_DOMAIN="satoshipay" +NAME="OBSRVR Validator 1" +PUBLIC_KEY="GDRCZ4IPJR7V3HK4GR45CRTE72SDAOZUF2TDBQ5E5IGWC4KM5TSKU2LS" +ADDRESS="core-live-1.nodeswithobsrvr.co:11625" +HISTORY="curl -sf https://history-1.nodeswithobsrvr.co/obsrvr-core-1/{0} -o {1}" +HOME_DOMAIN="stellar.withobsrvr.com" [[VALIDATORS]] -NAME="satoshipay_sg" -PUBLIC_KEY="GBJQUIXUO4XSNPAUT6ODLZUJRV2NPXYASKUBY4G5MYP3M47PCVI55MNT" -ADDRESS="stellar-sg-sin.satoshipay.io:11625" -HISTORY="curl -sf https://stellar-history-sg-sin.satoshipay.io/{0} -o {1}" -HOME_DOMAIN="satoshipay" +NAME="OBSRVR Validator 2" +PUBLIC_KEY="GA2PU4UGMLSFUXGZATHPTDXXX7FOHBAQC57RSJCQUN72WFKTD6CEPQSF" +ADDRESS="core-live-2.nodeswithobsrvr.co:11625" +HISTORY="curl -sf https://history-2.nodeswithobsrvr.co/obsrvr-core-2/{0} -o {1}" +HOME_DOMAIN="stellar.withobsrvr.com" [[VALIDATORS]] -NAME="satoshipay_us" -PUBLIC_KEY="GAK6Z5UVGUVSEK6PEOCAYJISTT5EJBB34PN3NOLEQG2SUKXRVV2F6HZY" -ADDRESS="stellar-us-iowa.satoshipay.io:11625" -HISTORY="curl -sf https://stellar-history-us-iowa.satoshipay.io/{0} -o {1}" -HOME_DOMAIN="satoshipay" +NAME="OBSRVR Validator 3" +PUBLIC_KEY="GACM6GIRMLXBBZIYJXBDTAEYZ2GJP3JJP5G5K4WDPBS6QFHPJNK6S2FB" +ADDRESS="core-live-3.nodeswithobsrvr.co:11625" +HISTORY="curl -sf https://history-3.nodeswithobsrvr.co/obsrvr-core-3/{0} -o {1}" +HOME_DOMAIN="stellar.withobsrvr.com" [[VALIDATORS]] NAME = "Gamma Node Validator" diff --git a/ingest/producer.go b/ingest/producer.go index 7d1a3ed452..619ae100fd 100644 --- a/ingest/producer.go +++ b/ingest/producer.go @@ -100,6 +100,7 @@ func ApplyLedgerMetadata(ledgerRange ledgerbackend.Range, if err != nil { return fmt.Errorf("failed to create datastore: %w", err) } + defer dataStore.Close() schema, err := datastore.LoadSchema(context.Background(), dataStore, publisherConfig.DataStoreConfig) if err != nil { @@ -116,6 +117,8 @@ func ApplyLedgerMetadata(ledgerRange ledgerbackend.Range, ledgerBackend = ledgerbackend.WithMetrics(ledgerBackend, publisherConfig.Registry, publisherConfig.RegistryNamespace) } + defer ledgerBackend.Close() + if ledgerRange.Bounded() && ledgerRange.To() <= ledgerRange.From() { return fmt.Errorf("invalid end value for bounded range, must be greater than start") } @@ -125,7 +128,9 @@ func ApplyLedgerMetadata(ledgerRange ledgerbackend.Range, } from := max(2, ledgerRange.From()) - ledgerBackend.PrepareRange(ctx, ledgerRange) + if err = ledgerBackend.PrepareRange(ctx, ledgerRange); err != nil { + return fmt.Errorf("failed to prepare range: %w", err) + } for ledgerSeq := from; ledgerSeq <= ledgerRange.To() || !ledgerRange.Bounded(); ledgerSeq++ { var ledgerCloseMeta xdr.LedgerCloseMeta diff --git a/ingest/producer_test.go b/ingest/producer_test.go index f60e98201b..53732fa733 100644 --- a/ingest/producer_test.go +++ b/ingest/producer_test.go @@ -117,6 +117,7 @@ func TestBSBProducerFnConfigError(t *testing.T) { mockDataStore.On("GetFile", mock.Anything, ".config.json"). Return(io.NopCloser(bytes.NewReader(configManifestJSON(t))), int64(-1), nil).Once() mockDataStore.On("ListFilePaths", mock.Anything, datastore.ListFileOptions{}).Return(nil, nil) + mockDataStore.On("Close").Return(nil).Once() datastoreFactory = func(_ context.Context, _ datastore.DataStoreConfig) (datastore.DataStore, error) { return mockDataStore, nil @@ -137,6 +138,7 @@ func TestBSBProducerFnInvalidRange(t *testing.T) { mockDataStore.On("GetFile", mock.Anything, ".config.json"). Return(io.NopCloser(bytes.NewReader(configManifestJSON(t))), int64(-1), nil).Once() mockDataStore.On("ListFilePaths", mock.Anything, datastore.ListFileOptions{}).Return(nil, nil) + mockDataStore.On("Close").Return(nil).Once() appCallback := func(lcm xdr.LedgerCloseMeta) error { return nil @@ -169,6 +171,7 @@ func TestBSBProducerFnGetLedgerError(t *testing.T) { // don't assert on it mockDataStore.On("GetFile", mock.Anything, "FFFFFFFC--3.xdr.zst").Return(makeSingleLCMBatch(3), int64(-1), nil).Maybe() mockDataStore.On("ListFilePaths", mock.Anything, datastore.ListFileOptions{}).Return(nil, nil) + mockDataStore.On("Close").Return(nil).Once() appCallback := func(lcm xdr.LedgerCloseMeta) error { return nil @@ -203,7 +206,11 @@ func TestBSBProducerFnCallbackError(t *testing.T) { DataStoreConfig: datastore.DataStoreConfig{}, BufferedStorageConfig: DefaultBufferedStorageBackendConfig(1), } - mockDataStore := createMockdataStore(t, 2, 3, 64000) + mockDataStore := createMockdataStore(t, 2, 2, 64000) + // The callback fails on ledger 2, so the multi-worker buffer may or may not + // get far enough to prefetch ledger 3 — allow the fetch without requiring it. + mockDataStore.On("GetFile", mock.Anything, fmt.Sprintf("FFFFFFFF--0-63999/%08X--3.xdr.zst", math.MaxUint32-3)). + Return(makeSingleLCMBatch(3), int64(-1), nil).Maybe() appCallback := func(lcm xdr.LedgerCloseMeta) error { return errors.New("uhoh") @@ -217,6 +224,7 @@ func TestBSBProducerFnCallbackError(t *testing.T) { "received an error from callback invocation") } +// Serves ledgers start..end, requiring exactly one fetch of each. func createMockdataStore(t *testing.T, start, end, partitionSize uint32) *datastore.MockDataStore { mockDataStore := new(datastore.MockDataStore) @@ -233,11 +241,13 @@ func createMockdataStore(t *testing.T, start, end, partitionSize uint32) *datast mockDataStore.On("GetFile", mock.Anything, ".config.json"). Return(io.NopCloser(bytes.NewReader(configJSON)), int64(-1), nil).Once() mockDataStore.On("ListFilePaths", mock.Anything, datastore.ListFileOptions{}).Return(nil, nil) + mockDataStore.On("Close").Return(nil).Once() partition := partitionSize - 1 for i := start; i <= end; i++ { objectName := fmt.Sprintf("FFFFFFFF--0-%d/%08X--%d.xdr.zst", partition, math.MaxUint32-i, i) - mockDataStore.On("GetFile", mock.Anything, objectName).Return(makeSingleLCMBatch(i), int64(-1), nil).Once() + mockDataStore.On("GetFile", mock.Anything, objectName). + Return(makeSingleLCMBatch(i), int64(-1), nil).Once() } t.Cleanup(func() { diff --git a/processors/token_transfer/token_transfer_processor.go b/processors/token_transfer/token_transfer_processor.go index 448d45ddd2..83608314e2 100644 --- a/processors/token_transfer/token_transfer_processor.go +++ b/processors/token_transfer/token_transfer_processor.go @@ -741,9 +741,10 @@ func (p *EventsProcessor) generateEventsForRevokedTrustlines(tx ingest.LedgerTra assetInCb := cbsCreatedByThisLp[0].Asset // The asset that needs to be burned is the one that is the OPPOSITE of the asset in the CB, so find that in the LP + // Equals, not ==: xdr.Asset's alphanum arms are pointers, so == compares identity. var burnedAsset xdr.Asset var burnedAmount xdr.Int64 - if assetInCb == lp.assetA { + if assetInCb.Equals(lp.assetA) { burnedAsset = lp.assetB burnedAmount = lp.amountChangeForAssetB } else { diff --git a/processors/token_transfer/token_transfer_processor_test.go b/processors/token_transfer/token_transfer_processor_test.go index 39b5ef1608..aa1876e759 100644 --- a/processors/token_transfer/token_transfer_processor_test.go +++ b/processors/token_transfer/token_transfer_processor_test.go @@ -339,6 +339,7 @@ var ( return xdr.LedgerEntryChange{ Type: xdr.LedgerEntryChangeTypeLedgerEntryRemoved, Removed: &xdr.LedgerKey{ + Type: xdr.LedgerEntryTypeClaimableBalance, ClaimableBalance: &xdr.LedgerKeyClaimableBalance{ BalanceId: cbId, }, @@ -392,6 +393,7 @@ var ( return xdr.LedgerEntryChange{ Type: xdr.LedgerEntryChangeTypeLedgerEntryRemoved, Removed: &xdr.LedgerKey{ + Type: xdr.LedgerEntryTypeLiquidityPool, LiquidityPool: &xdr.LedgerKeyLiquidityPool{ LiquidityPoolId: lpId, }, @@ -602,13 +604,24 @@ type testFixture struct { wantErr bool } +// Re-decodes the meta so each xdr.Asset gets its own allocation, as in +// production. Fixtures share package-level assets, which aliases their pointers. +func reEncodeMeta(t *testing.T, tx ingest.LedgerTransaction) ingest.LedgerTransaction { + t.Helper() + raw, err := tx.UnsafeMeta.MarshalBinary() + require.NoError(t, err) + out := tx + require.NoError(t, out.UnsafeMeta.UnmarshalBinary(raw)) + return out +} + // RunTokenTransferEventTests runs a standard set of tests for token transfer event processing func runTokenTransferEventTests(t *testing.T, tests []testFixture) { for _, fixture := range tests { ttp := NewEventsProcessor(someNetworkPassphrase) t.Run(fixture.name, func(t *testing.T) { events, err := ttp.EventsFromOperation( - fixture.tx, + reEncodeMeta(t, fixture.tx), fixture.opIndex, fixture.op, fixture.opResult, diff --git a/txnbuild/CHANGELOG.md b/txnbuild/CHANGELOG.md index 101701c8ff..50545c7fa7 100644 --- a/txnbuild/CHANGELOG.md +++ b/txnbuild/CHANGELOG.md @@ -8,6 +8,10 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). ### Breaking changes * `SetOpSourceAccount` now returns an `error` instead of silently ignoring invalid source account addresses. All `BuildXDR()` methods propagate this error. ([#5912](https://github.com/stellar/go-stellar-sdk/pull/5912)) +* Liquidity pool asset pairs are validated strictly ([#5974](https://github.com/stellar/go-stellar-sdk/pull/5974)): + * `LiquidityPoolParameters.ToXDR`, `NewLiquidityPoolId`, `NewLiquidityPoolDeposit`, and `NewLiquidityPoolWithdraw` now require `AssetA < AssetB` in the protocol's order (raw issuer key, not strkey text). Reversed or identical pairs — which previously could build operations that stellar-core rejects with `CHANGE_TRUST_MALFORMED` — now error at build time, with the message changed from `AssetA must be <= AssetB` to `AssetA must be < AssetB`. + * Asset sort order via `Assets`/`LessThan` changes accordingly, and `NativeAsset.LessThan` no longer reports a native asset as less than another native asset. + * `NewLiquidityPoolId`, `NewLiquidityPoolDeposit`, and `NewLiquidityPoolWithdraw` validate ordering after XDR conversion, so a malformed asset now returns the conversion error rather than an ordering error ([#5978](https://github.com/stellar/go-stellar-sdk/pull/5978)). ## [11.0.0](https://github.com/stellar/go-stellar-sdk/releases/tag/horizonclient-v11.0.0) - 2023-03-29 diff --git a/txnbuild/asset.go b/txnbuild/asset.go index 24a358cea1..16cf40715a 100644 --- a/txnbuild/asset.go +++ b/txnbuild/asset.go @@ -61,8 +61,9 @@ func (na NativeAsset) GetCode() string { return "" } // GetIssuer for NativeAsset returns an empty string (XLM doesn't have an issuer). func (na NativeAsset) GetIssuer() string { return "" } -// LessThan returns true if this asset sorts before some other asset. -func (na NativeAsset) LessThan(other Asset) bool { return true } +// LessThan returns true if this asset sorts strictly before the other. Native +// sorts before every credit asset, but not before another native asset. +func (na NativeAsset) LessThan(other Asset) bool { return !other.IsNative() } // ToXDR for NativeAsset produces a corresponding XDR asset. func (na NativeAsset) ToXDR() (xdr.Asset, error) { diff --git a/txnbuild/helpers.go b/txnbuild/helpers.go index 4a954f2774..9ca69715ec 100644 --- a/txnbuild/helpers.go +++ b/txnbuild/helpers.go @@ -147,8 +147,13 @@ func validateChangeTrustAsset(asset ChangeTrustAsset) error { if err != nil { return err } else if assetType == AssetTypePoolShare { - // No issuer for these to validate. - return nil + // No issuer to validate, but ToXDR checks the asset pair is ordered. + params, ok := asset.GetLiquidityPoolParameters() + if !ok { + return errors.New("liquidity pool share asset has no pool parameters") + } + _, err = params.ToXDR() + return err } err = validateStellarPublicKey(asset.GetIssuer()) diff --git a/txnbuild/liquidity_pool_deposit.go b/txnbuild/liquidity_pool_deposit.go index 1b213d4257..b0d603c5e4 100644 --- a/txnbuild/liquidity_pool_deposit.go +++ b/txnbuild/liquidity_pool_deposit.go @@ -28,10 +28,6 @@ func NewLiquidityPoolDeposit( minPrice, maxPrice xdr.Price, ) (LiquidityPoolDeposit, error) { - if b.Asset.LessThan(a.Asset) { - return LiquidityPoolDeposit{}, errors.New("AssetA must be <= AssetB") - } - poolId, err := NewLiquidityPoolId(a.Asset, b.Asset) if err != nil { return LiquidityPoolDeposit{}, err diff --git a/txnbuild/liquidity_pool_deposit_test.go b/txnbuild/liquidity_pool_deposit_test.go index 11d81c4e21..1804901219 100644 --- a/txnbuild/liquidity_pool_deposit_test.go +++ b/txnbuild/liquidity_pool_deposit_test.go @@ -46,7 +46,18 @@ func TestNewLiquidityPoolDeposit(t *testing.T) { price.MustParse("0.3"), price.MustParse("0.4"), ) - require.EqualError(t, err, "AssetA must be <= AssetB") + require.EqualError(t, err, "AssetA must be < AssetB") + }) + + t.Run("malformed asset", func(t *testing.T) { + _, err := NewLiquidityPoolDeposit( + "GB7BDSZU2Y27LYNLALKKALB52WS2IZWYBDGY6EQBLEED3TJOCVMZRH7H", + AssetAmount{CreditAsset{Code: "EUR", Issuer: "malformed"}, "0.1000000"}, + AssetAmount{assetB, "0.2000000"}, + price.MustParse("0.3"), + price.MustParse("0.4"), + ) + require.ErrorContains(t, err, "failed to build XDR AssetA ID") }) } diff --git a/txnbuild/liquidity_pool_id.go b/txnbuild/liquidity_pool_id.go index 6c2404b096..1b164bed72 100644 --- a/txnbuild/liquidity_pool_id.go +++ b/txnbuild/liquidity_pool_id.go @@ -2,8 +2,6 @@ package txnbuild import ( - "fmt" - "github.com/stellar/go-stellar-sdk/support/errors" "github.com/stellar/go-stellar-sdk/xdr" ) @@ -12,10 +10,6 @@ import ( type LiquidityPoolId [32]byte func NewLiquidityPoolId(a, b Asset) (LiquidityPoolId, error) { - if b.LessThan(a) { - return LiquidityPoolId{}, fmt.Errorf("AssetA must be <= AssetB") - } - xdrAssetA, err := a.ToXDR() if err != nil { return LiquidityPoolId{}, errors.Wrap(err, "failed to build XDR AssetA ID") @@ -26,9 +20,12 @@ func NewLiquidityPoolId(a, b Asset) (LiquidityPoolId, error) { return LiquidityPoolId{}, errors.Wrap(err, "failed to build XDR AssetB ID") } + // xdr.NewPoolId enforces the pool ordering invariant (strictly AssetA < + // AssetB). Its error is returned as-is so callers see the same message the + // XDR layer produces. id, err := xdr.NewPoolId(xdrAssetA, xdrAssetB, xdr.LiquidityPoolFeeV18) if err != nil { - return LiquidityPoolId{}, errors.Wrap(err, "failed to build XDR liquidity pool id") + return LiquidityPoolId{}, err } return LiquidityPoolId(id), nil } diff --git a/txnbuild/liquidity_pool_id_test.go b/txnbuild/liquidity_pool_id_test.go index 377fd2104b..0c7300a890 100644 --- a/txnbuild/liquidity_pool_id_test.go +++ b/txnbuild/liquidity_pool_id_test.go @@ -23,5 +23,5 @@ func TestNewLiquidityPoolId(t *testing.T) { // Wrong asset id order should fail. If users mess this up, and we were to // silently fix it they could set the wrong MaxAmounts when depositing. _, err = NewLiquidityPoolId(b, a) - assert.EqualError(t, err, "AssetA must be <= AssetB") + assert.EqualError(t, err, "AssetA must be < AssetB") } diff --git a/txnbuild/liquidity_pool_ordering_test.go b/txnbuild/liquidity_pool_ordering_test.go new file mode 100644 index 0000000000..608710f38e --- /dev/null +++ b/txnbuild/liquidity_pool_ordering_test.go @@ -0,0 +1,147 @@ +package txnbuild + +import ( + "testing" + + "github.com/stellar/go-stellar-sdk/keypair" + "github.com/stellar/go-stellar-sdk/xdr" + + "github.com/stretchr/testify/require" +) + +func orderedPair(t *testing.T, a, b CreditAsset) (CreditAsset, CreditAsset) { + t.Helper() + aXDR, err := a.ToXDR() + require.NoError(t, err) + bXDR, err := b.ToXDR() + require.NoError(t, err) + if bXDR.LessThan(aXDR) { + return b, a + } + return a, b +} + +func TestChangeTrustPoolParamsCanAlwaysBeReadBack(t *testing.T) { + built := 0 + for i := 0; i < 2000; i++ { + a := CreditAsset{Code: "USD", Issuer: keypair.MustRandom().Address()} + b := CreditAsset{Code: "USD", Issuer: keypair.MustRandom().Address()} + + for _, pair := range [][2]CreditAsset{{a, b}, {b, a}} { + op := &ChangeTrust{ + Line: LiquidityPoolShareChangeTrustAsset{ + LiquidityPoolParameters: LiquidityPoolParameters{ + AssetA: pair[0], + AssetB: pair[1], + Fee: LiquidityPoolFeeV18, + }, + }, + Limit: MaxTrustlineLimit, + } + + xdrOp, err := op.BuildXDR() + if err != nil { + continue // refusing an out-of-order pair is correct + } + built++ + + cp := xdrOp.Body.MustChangeTrustOp().Line.MustLiquidityPool().ConstantProduct + _, err = xdr.NewPoolId(cp.AssetA, cp.AssetB, cp.Fee) + require.NoError(t, err, + "txnbuild produced pool parameters whose pool id cannot be derived") + } + } + require.Equal(t, 2000, built, + "exactly one order of each distinct pair must build") +} + +func TestChangeTrustRejectsOutOfOrderPoolParams(t *testing.T) { + first, second := orderedPair(t, + CreditAsset{Code: "USD", Issuer: keypair.MustRandom().Address()}, + CreditAsset{Code: "USD", Issuer: keypair.MustRandom().Address()}, + ) + + inOrder := &ChangeTrust{ + Line: LiquidityPoolShareChangeTrustAsset{ + LiquidityPoolParameters: LiquidityPoolParameters{ + AssetA: first, AssetB: second, Fee: LiquidityPoolFeeV18, + }, + }, + Limit: MaxTrustlineLimit, + } + require.NoError(t, inOrder.Validate()) + _, err := inOrder.BuildXDR() + require.NoError(t, err) + + reversed := &ChangeTrust{ + Line: LiquidityPoolShareChangeTrustAsset{ + LiquidityPoolParameters: LiquidityPoolParameters{ + AssetA: second, AssetB: first, Fee: LiquidityPoolFeeV18, + }, + }, + Limit: MaxTrustlineLimit, + } + require.Error(t, reversed.Validate()) + _, err = reversed.BuildXDR() + require.Error(t, err) +} + +func TestChangeTrustRejectsIdenticalPoolAssets(t *testing.T) { + asset := CreditAsset{Code: "USD", Issuer: keypair.MustRandom().Address()} + + op := &ChangeTrust{ + Line: LiquidityPoolShareChangeTrustAsset{ + LiquidityPoolParameters: LiquidityPoolParameters{ + AssetA: asset, AssetB: asset, Fee: LiquidityPoolFeeV18, + }, + }, + Limit: MaxTrustlineLimit, + } + require.Error(t, op.Validate()) + _, err := op.BuildXDR() + require.Error(t, err) + + _, err = LiquidityPoolParameters{ + AssetA: asset, AssetB: asset, Fee: LiquidityPoolFeeV18, + }.ToXDR() + require.Error(t, err) + + native := NativeAsset{} + _, err = LiquidityPoolParameters{ + AssetA: native, AssetB: native, Fee: LiquidityPoolFeeV18, + }.ToXDR() + require.Error(t, err, "two native assets are also not a valid pair") +} + +func TestLiquidityPoolParametersToXDRRejectsOutOfOrderPair(t *testing.T) { + first, second := orderedPair(t, + CreditAsset{Code: "EUR", Issuer: keypair.MustRandom().Address()}, + CreditAsset{Code: "EUR", Issuer: keypair.MustRandom().Address()}, + ) + + _, err := LiquidityPoolParameters{ + AssetA: first, AssetB: second, Fee: LiquidityPoolFeeV18, + }.ToXDR() + require.NoError(t, err) + + _, err = LiquidityPoolParameters{ + AssetA: second, AssetB: first, Fee: LiquidityPoolFeeV18, + }.ToXDR() + require.Error(t, err) +} + +func TestChangeTrustAcceptsNativePairedPool(t *testing.T) { + op := &ChangeTrust{ + Line: LiquidityPoolShareChangeTrustAsset{ + LiquidityPoolParameters: LiquidityPoolParameters{ + AssetA: NativeAsset{}, + AssetB: CreditAsset{Code: "USD", Issuer: keypair.MustRandom().Address()}, + Fee: LiquidityPoolFeeV18, + }, + }, + Limit: MaxTrustlineLimit, + } + require.NoError(t, op.Validate()) + _, err := op.BuildXDR() + require.NoError(t, err) +} diff --git a/txnbuild/liquidity_pool_parameters.go b/txnbuild/liquidity_pool_parameters.go index 2db58d4899..af6e67a010 100644 --- a/txnbuild/liquidity_pool_parameters.go +++ b/txnbuild/liquidity_pool_parameters.go @@ -28,6 +28,12 @@ func (lpi LiquidityPoolParameters) ToXDR() (xdr.LiquidityPoolParameters, error) return xdr.LiquidityPoolParameters{}, errors.Wrap(err, "failed to build XDR AssetB ID") } + // AssetA must sort strictly before AssetB — two identical assets are not a + // valid pool pair, so the test is "not less than" rather than "greater than". + if !xdrAssetA.LessThan(xdrAssetB) { + return xdr.LiquidityPoolParameters{}, errors.New("AssetA must be < AssetB") + } + return xdr.LiquidityPoolParameters{ Type: xdr.LiquidityPoolTypeLiquidityPoolConstantProduct, ConstantProduct: &xdr.LiquidityPoolConstantProductParameters{ diff --git a/txnbuild/liquidity_pool_withdraw.go b/txnbuild/liquidity_pool_withdraw.go index 58604066cd..f2409a65a3 100644 --- a/txnbuild/liquidity_pool_withdraw.go +++ b/txnbuild/liquidity_pool_withdraw.go @@ -26,10 +26,6 @@ func NewLiquidityPoolWithdraw( a, b AssetAmount, amount string, ) (LiquidityPoolWithdraw, error) { - if b.Asset.LessThan(a.Asset) { - return LiquidityPoolWithdraw{}, errors.New("AssetA must be <= AssetB") - } - poolId, err := NewLiquidityPoolId(a.Asset, b.Asset) if err != nil { return LiquidityPoolWithdraw{}, err diff --git a/txnbuild/liquidity_pool_withdraw_test.go b/txnbuild/liquidity_pool_withdraw_test.go index d2b172392e..3b2125e0cc 100644 --- a/txnbuild/liquidity_pool_withdraw_test.go +++ b/txnbuild/liquidity_pool_withdraw_test.go @@ -41,7 +41,17 @@ func TestNewLiquidityPoolWithdraw(t *testing.T) { AssetAmount{assetA, "0.2000000"}, "52.5", ) - require.EqualError(t, err, "AssetA must be <= AssetB") + require.EqualError(t, err, "AssetA must be < AssetB") + }) + + t.Run("malformed asset", func(t *testing.T) { + _, err := NewLiquidityPoolWithdraw( + "GB7BDSZU2Y27LYNLALKKALB52WS2IZWYBDGY6EQBLEED3TJOCVMZRH7H", + AssetAmount{CreditAsset{Code: "EUR", Issuer: "malformed"}, "0.1000000"}, + AssetAmount{assetB, "0.2000000"}, + "52.5", + ) + require.ErrorContains(t, err, "failed to build XDR AssetA ID") }) } diff --git a/xdr/asset.go b/xdr/asset.go index 5931b276af..beeb7d95dc 100644 --- a/xdr/asset.go +++ b/xdr/asset.go @@ -1,6 +1,7 @@ package xdr import ( + "bytes" "crypto/sha256" "errors" "fmt" @@ -436,16 +437,29 @@ func (a *Asset) GetIssuerAccountId() (AccountId, error) { return addr, nil } +// LessThan orders assets the way the XDR encoding does: by type, then by the +// NUL-padded code bytes, then by the raw 32-byte issuer key. The base32 "G..." +// strkey is not order preserving, so comparing issuers as strkey text would +// give a different ordering. func (a *Asset) LessThan(b Asset) bool { if a.Type != b.Type { return int32(a.Type) < int32(b.Type) } + if a.Type == AssetTypeAssetTypeNative { + return false + } + if a.GetCode() != b.GetCode() { return a.GetCode() < b.GetCode() } - return a.GetIssuer() < b.GetIssuer() + // GetIssuerAccountId only errors for native assets, which returned above. + aIssuer, _ := a.GetIssuerAccountId() + bIssuer, _ := b.GetIssuerAccountId() + aKey := aIssuer.MustEd25519() + bKey := bIssuer.MustEd25519() + return bytes.Compare(aKey[:], bKey[:]) < 0 } // ContractID returns the expected Stellar Asset Contract id for the given diff --git a/xdr/asset_ordering_test.go b/xdr/asset_ordering_test.go new file mode 100644 index 0000000000..6d34a744ce --- /dev/null +++ b/xdr/asset_ordering_test.go @@ -0,0 +1,110 @@ +package xdr_test + +import ( + "bytes" + "testing" + + "github.com/stellar/go-stellar-sdk/keypair" + . "github.com/stellar/go-stellar-sdk/xdr" + + "github.com/stretchr/testify/require" +) + +func TestAssetLessThanMatchesXDROrdering(t *testing.T) { + for i := 0; i < 500; i++ { + issuer1 := keypair.MustRandom().Address() + issuer2 := keypair.MustRandom().Address() + + assets := []Asset{ + MustNewNativeAsset(), + MustNewCreditAsset("A", issuer1), + MustNewCreditAsset("USD", issuer1), + MustNewCreditAsset("USD", issuer2), + MustNewCreditAsset("ZZZZ", issuer1), + MustNewCreditAsset("LONGASSET12", issuer1), + MustNewCreditAsset("LONGASSET12", issuer2), + MustNewCreditAsset("AAAAAAAAAAAA", issuer2), + } + + for _, a := range assets { + for _, b := range assets { + aBytes, err := a.MarshalBinary() + require.NoError(t, err) + bBytes, err := b.MarshalBinary() + require.NoError(t, err) + + require.Equal(t, bytes.Compare(aBytes, bBytes) < 0, a.LessThan(b), + "LessThan disagrees with the XDR encoding for %s vs %s", + a.StringCanonical(), b.StringCanonical()) + } + } + } +} + +func TestAssetLessThanComparesIssuerBytesNotStrkey(t *testing.T) { + a := MustNewCreditAsset("USD", "GCXHWP6ILITHEZWVNCCTPJCT7ZIQ2JGKJH7XXR4VYI7PMAGOIHBMHHHM") + b := MustNewCreditAsset("USD", "GC3BT2M7I2M5PJWE4VWYRVSOHSE6YBD2QKWVJH4TX7GSA3UHYCDH2YCD") + + require.Less(t, b.GetIssuer(), a.GetIssuer(), + "precondition: compared as strkey text, b sorts first") + + aIssuer := a.MustAlphaNum4().Issuer.MustEd25519() + bIssuer := b.MustAlphaNum4().Issuer.MustEd25519() + require.Negative(t, bytes.Compare(aIssuer[:], bIssuer[:]), + "precondition: compared as raw bytes, a sorts first") + + require.True(t, a.LessThan(b)) + require.False(t, b.LessThan(a)) + + _, err := NewPoolId(a, b, LiquidityPoolFeeV18) + require.NoError(t, err, "the byte-ordered pair must be accepted") + + _, err = NewPoolId(b, a, LiquidityPoolFeeV18) + require.Error(t, err, "the strkey-ordered pair must be rejected") +} + +func TestNewPoolIdRequiresStrictOrdering(t *testing.T) { + issuer := keypair.MustRandom().Address() + asset := MustNewCreditAsset("USD", issuer) + native := MustNewNativeAsset() + + _, err := NewPoolId(native, asset, LiquidityPoolFeeV18) + require.NoError(t, err) + + _, err = NewPoolId(asset, asset, LiquidityPoolFeeV18) + require.Error(t, err, "a pool cannot pair an asset with itself") + + _, err = NewPoolId(native, native, LiquidityPoolFeeV18) + require.Error(t, err, "two native assets are not a valid pair either") + + _, err = NewPoolId(asset, native, LiquidityPoolFeeV18) + require.Error(t, err, "a reversed pair is rejected") +} + +func TestAssetLessThanIsAStrictOrder(t *testing.T) { + for i := 0; i < 2000; i++ { + a := MustNewCreditAsset("USD", keypair.MustRandom().Address()) + b := MustNewCreditAsset("USD", keypair.MustRandom().Address()) + + require.False(t, a.LessThan(a)) + if a.LessThan(b) { + require.False(t, b.LessThan(a)) + } + } +} + +func TestAssetLessThanOrdersByTypeThenCode(t *testing.T) { + issuer := keypair.MustRandom().Address() + + native := MustNewNativeAsset() + alphaNum4 := MustNewCreditAsset("USD", issuer) + alphaNum12 := MustNewCreditAsset("LONGASSET12", issuer) + + require.True(t, native.LessThan(alphaNum4)) + require.True(t, alphaNum4.LessThan(alphaNum12), + "alphanum4 sorts before alphanum12 regardless of code") + + aaa := MustNewCreditAsset("AAA", issuer) + bbb := MustNewCreditAsset("BBB", issuer) + require.True(t, aaa.LessThan(bbb)) +} diff --git a/xdr/pool_id.go b/xdr/pool_id.go index 2e1e72370f..8a60cabb76 100644 --- a/xdr/pool_id.go +++ b/xdr/pool_id.go @@ -7,12 +7,13 @@ import ( "github.com/stellar/go-stellar-sdk/support/errors" ) +// NewPoolId requires a and b in protocol order, strictly a < b. The id depends +// on that order, so a reversed or identical pair is rejected, not reordered. func NewPoolId(a, b Asset, fee Int32) (PoolId, error) { - if b.LessThan(a) { + if !a.LessThan(b) { return PoolId{}, errors.New("AssetA must be < AssetB") } - // Assume the assets are already sorted. params := LiquidityPoolParameters{ Type: LiquidityPoolTypeLiquidityPoolConstantProduct, ConstantProduct: &LiquidityPoolConstantProductParameters{