Include empty paths in path finding response (#4137)

stellar · Dec 10, 2021 · e80796c · e80796c
1 parent 6c6503e
commit e80796c
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 27 deletions.
diff --git a/exp/orderbook/graph.go b/exp/orderbook/graph.go
@@ -692,8 +692,37 @@ func (graph *OrderBookGraph) FindPaths(
 	validateSourceBalance bool,
 	maxAssetsPerPath int,
 	includePools bool,
+) ([]Path, uint32, error) {
+	paths, lastLedger, err := graph.findPathsWithLock(
+		ctx, maxPathLength, destinationAsset, destinationAmount, sourceAccountID, sourceAssets, sourceAssetBalances,
+		validateSourceBalance, includePools,
+	)
+	if err != nil {
+		return nil, lastLedger, errors.Wrap(err, "could not determine paths")
+	}
+
+	paths, err = sortAndFilterPaths(
+		paths,
+		maxAssetsPerPath,
+		sortBySourceAsset,
+	)
+	return paths, lastLedger, err
+}
+
+func (graph *OrderBookGraph) findPathsWithLock(
+	ctx context.Context,
+	maxPathLength int,
+	destinationAsset xdr.Asset,
+	destinationAmount xdr.Int64,
+	sourceAccountID *xdr.AccountId,
+	sourceAssets []xdr.Asset,
+	sourceAssetBalances []xdr.Int64,
+	validateSourceBalance bool,
+	includePools bool,
 ) ([]Path, uint32, error) {
 	graph.lock.RLock()
+	defer graph.lock.RUnlock()
+
 	destinationAssetString := destinationAsset.String()
 	sourceAssetsMap := make(map[int32]xdr.Int64, len(sourceAssets))
 	for i, sourceAsset := range sourceAssets {
@@ -706,7 +735,6 @@ func (graph *OrderBookGraph) FindPaths(
 	}
 	destinationAssetID, ok := graph.assetStringToID[destinationAssetString]
 	if !ok || len(sourceAssetsMap) == 0 {
-		graph.lock.RUnlock()
 		return []Path{}, graph.lastLedger, nil
 	}
 	searchState := &sellingGraphSearchState{
@@ -726,18 +754,7 @@ func (graph *OrderBookGraph) FindPaths(
 		destinationAssetID,
 		destinationAmount,
 	)
-	lastLedger := graph.lastLedger
-	graph.lock.RUnlock()
-	if err != nil {
-		return nil, lastLedger, errors.Wrap(err, "could not determine paths")
-	}
-
-	paths, err := sortAndFilterPaths(
-		searchState.paths,
-		maxAssetsPerPath,
-		sortBySourceAsset,
-	)
-	return paths, lastLedger, err
+	return searchState.paths, graph.lastLedger, err
 }
 
 type sortablePaths struct {
@@ -773,8 +790,33 @@ func (graph *OrderBookGraph) FindFixedPaths(
 	destinationAssets []xdr.Asset,
 	maxAssetsPerPath int,
 	includePools bool,
+) ([]Path, uint32, error) {
+	paths, lastLedger, err := graph.findFixedPathsWithLock(
+		ctx, maxPathLength, sourceAsset, amountToSpend, destinationAssets, includePools,
+	)
+	if err != nil {
+		return nil, lastLedger, errors.Wrap(err, "could not determine paths")
+	}
+
+	paths, err = sortAndFilterPaths(
+		paths,
+		maxAssetsPerPath,
+		sortByDestinationAsset,
+	)
+	return paths, lastLedger, err
+}
+
+func (graph *OrderBookGraph) findFixedPathsWithLock(
+	ctx context.Context,
+	maxPathLength int,
+	sourceAsset xdr.Asset,
+	amountToSpend xdr.Int64,
+	destinationAssets []xdr.Asset,
+	includePools bool,
 ) ([]Path, uint32, error) {
 	graph.lock.RLock()
+	defer graph.lock.RUnlock()
+
 	target := make(map[int32]bool, len(destinationAssets))
 	for _, destinationAsset := range destinationAssets {
 		destinationAssetString := destinationAsset.String()
@@ -788,7 +830,6 @@ func (graph *OrderBookGraph) FindFixedPaths(
 	sourceAssetString := sourceAsset.String()
 	sourceAssetID, ok := graph.assetStringToID[sourceAssetString]
 	if !ok || len(target) == 0 {
-		graph.lock.RUnlock()
 		return []Path{}, graph.lastLedger, nil
 	}
 	searchState := &buyingGraphSearchState{
@@ -806,18 +847,7 @@ func (graph *OrderBookGraph) FindFixedPaths(
 		sourceAssetID,
 		amountToSpend,
 	)
-	lastLedger := graph.lastLedger
-	graph.lock.RUnlock()
-	if err != nil {
-		return nil, lastLedger, errors.Wrap(err, "could not determine paths")
-	}
-
-	paths, err := sortAndFilterPaths(
-		searchState.paths,
-		maxAssetsPerPath,
-		sortByDestinationAsset,
-	)
-	return paths, lastLedger, err
+	return searchState.paths, graph.lastLedger, err
 }
 
 // compareSourceAsset will group payment paths by `SourceAsset`

diff --git a/exp/orderbook/graph_test.go b/exp/orderbook/graph_test.go
@@ -1579,10 +1579,12 @@ func TestFindPaths(t *testing.T) {
 		[]xdr.Asset{
 			yenAsset,
 			usdAsset,
+			nativeAsset,
 		},
 		[]xdr.Int64{
 			100000,
 			60000,
+			100000,
 		},
 		true,
 		5,
@@ -1618,6 +1620,15 @@ func TestFindPaths(t *testing.T) {
 			DestinationAsset:  nativeAsset.String(),
 			DestinationAmount: 20,
 		},
+		// include the empty path where xlm is transferred without any
+		// conversions
+		{
+			SourceAmount:      20,
+			SourceAsset:       nativeAsset.String(),
+			InteriorNodes:     []string{},
+			DestinationAsset:  nativeAsset.String(),
+			DestinationAmount: 20,
+		},
 	}
 
 	assert.NoError(t, err)
@@ -1873,7 +1884,7 @@ func TestFindPathsStartingAt(t *testing.T) {
 		5,
 		yenAsset,
 		5,
-		[]xdr.Asset{nativeAsset, usdAsset},
+		[]xdr.Asset{nativeAsset, usdAsset, yenAsset},
 		5,
 		true,
 	)
@@ -1895,6 +1906,15 @@ func TestFindPathsStartingAt(t *testing.T) {
 			DestinationAsset:  usdAsset.String(),
 			DestinationAmount: 20,
 		},
+		// include the empty path where yen is transferred without any
+		// conversions
+		{
+			SourceAmount:      5,
+			SourceAsset:       yenAsset.String(),
+			InteriorNodes:     []string{},
+			DestinationAsset:  yenAsset.String(),
+			DestinationAmount: 5,
+		},
 		{
 			SourceAmount: 5,
 			SourceAsset:  yenAsset.String(),

diff --git a/exp/orderbook/search.go b/exp/orderbook/search.go
@@ -138,6 +138,17 @@ func search(
 		asset: sourceAsset,
 		prev:  nil,
 	}
+	// Simple payments (e.g. payments where an asset is transferred from
+	// one account to another without any conversions into another asset)
+	// are also valid path payments. If the source asset is a valid
+	// destination asset we include the empty path in the response.
+	if state.includePath(sourceAsset, sourceAssetAmount) {
+		state.appendToPaths(
+			[]int32{sourceAsset},
+			sourceAsset,
+			sourceAssetAmount,
+		)
+	}
 
 	for i := 0; i < maxPathLength; i++ {
 		updatedAssets = updatedAssets[:0]