From dc39d63d8db4f8a1b919b771eaa9c53bdddb9208 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Fri, 31 Jul 2026 21:19:02 +0200 Subject: [PATCH] assets: avoid locking cache during RPC Restrict the asset-name cache mutex to map access so a slow QueryAssetStats call cannot block cached readers. Use an RWMutex for independent cache reads and add a concurrent regression test. --- assets/client.go | 25 ++++++++++--- assets/client_test.go | 84 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 5 deletions(-) diff --git a/assets/client.go b/assets/client.go index 2d963bb0..ef320af8 100644 --- a/assets/client.go +++ b/assets/client.go @@ -81,7 +81,7 @@ type TapdClient struct { rfqTimeoutSeconds uint32 assetNameCache map[string]string - assetNameMutex sync.Mutex + assetNameMutex sync.RWMutex cc *grpc.ClientConn } @@ -169,10 +169,8 @@ func (c *TapdClient) GetRfqForAsset(ctx context.Context, func (c *TapdClient) GetAssetName(ctx context.Context, assetId []byte) (string, error) { - c.assetNameMutex.Lock() - defer c.assetNameMutex.Unlock() assetIdStr := hex.EncodeToString(assetId) - if name, ok := c.assetNameCache[assetIdStr]; ok { + if name, ok := c.getCachedAssetName(assetIdStr); ok { return name, nil } @@ -198,11 +196,28 @@ func (c *TapdClient) GetAssetName(ctx context.Context, assetName = assetStats.AssetStats[0].Asset.AssetName } - c.assetNameCache[assetIdStr] = assetName + c.cacheAssetName(assetIdStr, assetName) return assetName, nil } +// getCachedAssetName returns an asset name from the cache. +func (c *TapdClient) getCachedAssetName(assetID string) (string, bool) { + c.assetNameMutex.RLock() + defer c.assetNameMutex.RUnlock() + + name, ok := c.assetNameCache[assetID] + return name, ok +} + +// cacheAssetName adds an asset name to the cache. +func (c *TapdClient) cacheAssetName(assetID, name string) { + c.assetNameMutex.Lock() + defer c.assetNameMutex.Unlock() + + c.assetNameCache[assetID] = name +} + // GetAssetPrice returns the price of an asset in satoshis. NOTE: this currently // uses the rfq process for the asset price. A future implementation should // use a price oracle to not spam a peer. diff --git a/assets/client_test.go b/assets/client_test.go index c23ab7f2..5fd2b2e1 100644 --- a/assets/client_test.go +++ b/assets/client_test.go @@ -1,6 +1,8 @@ package assets import ( + "context" + "encoding/hex" "encoding/pem" "math" "net/http" @@ -12,11 +14,38 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/lightninglabs/taproot-assets/taprpc/rfqrpc" + "github.com/lightninglabs/taproot-assets/taprpc/universerpc" "github.com/lightningnetwork/lnd/lnwire" "github.com/stretchr/testify/require" + "google.golang.org/grpc" "gopkg.in/macaroon.v2" ) +type blockingUniverseClient struct { + universerpc.UniverseClient + + queryStarted chan struct{} + releaseQuery chan struct{} +} + +func (b *blockingUniverseClient) QueryAssetStats(context.Context, + *universerpc.AssetStatsQuery, ...grpc.CallOption) ( + *universerpc.UniverseAssetStats, error) { + + close(b.queryStarted) + <-b.releaseQuery + + return &universerpc.UniverseAssetStats{ + AssetStats: []*universerpc.AssetStatsSnapshot{ + { + Asset: &universerpc.AssetStatsAsset{ + AssetName: "queried asset", + }, + }, + }, + }, nil +} + // TestDefaultTapdConfig tests that the default tapd connection paths match // tapd's mainnet defaults. func TestDefaultTapdConfig(t *testing.T) { @@ -84,6 +113,61 @@ func TestTapdConfigClientConn(t *testing.T) { ) } +// TestGetAssetNameCachedLookupNotBlocked verifies that a slow universe query +// for one asset does not prevent another caller from reading a cached name. +func TestGetAssetNameCachedLookupNotBlocked(t *testing.T) { + const cachedName = "cached asset" + + cachedAssetID := []byte{1} + queryStarted := make(chan struct{}) + releaseQuery := make(chan struct{}) + client := &TapdClient{ + UniverseClient: &blockingUniverseClient{ + queryStarted: queryStarted, + releaseQuery: releaseQuery, + }, + assetNameCache: map[string]string{ + hex.EncodeToString(cachedAssetID): cachedName, + }, + } + + queryResult := make(chan error, 1) + go func() { + _, err := client.GetAssetName(context.Background(), []byte{2}) + queryResult <- err + }() + + select { + case <-queryStarted: + case <-time.After(time.Second): + t.Fatal("universe query did not start") + } + + type nameResult struct { + name string + err error + } + cachedResult := make(chan nameResult, 1) + go func() { + name, err := client.GetAssetName( + context.Background(), cachedAssetID, + ) + cachedResult <- nameResult{name: name, err: err} + }() + + select { + case result := <-cachedResult: + require.NoError(t, result.err) + require.Equal(t, cachedName, result.name) + case <-time.After(time.Second): + close(releaseQuery) + t.Fatal("cached lookup blocked behind universe query") + } + + close(releaseQuery) + require.NoError(t, <-queryResult) +} + func TestGetPaymentMaxAmount(t *testing.T) { tests := []struct { satAmount btcutil.Amount