Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFQ: add decimals cache #2502

Merged
merged 4 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 45 additions & 53 deletions services/rfq/relayer/service/chainindexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"fmt"

"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
"github.com/synapsecns/sanguine/core/metrics"
"github.com/synapsecns/sanguine/services/rfq/contracts/fastbridge"
Expand Down Expand Up @@ -119,10 +120,10 @@
return nil
}

const ethDecimals = 18
var ethDecimals uint8 = 18

// getDecimals gets the decimals for the origin and dest tokens.
func (r *Relayer) getDecimals(parentCtx context.Context, bridgeTx fastbridge.IFastBridgeBridgeTransaction) (_ *decimalsRes, err error) {
func (r *Relayer) getDecimalsFromBridgeTx(parentCtx context.Context, bridgeTx fastbridge.IFastBridgeBridgeTransaction) (originDecimals *uint8, destDecimals *uint8, err error) {

Check warning on line 126 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L126

Added line #L126 was not covered by tests
ctx, span := r.metrics.Tracer().Start(parentCtx, "getDecimals", trace.WithAttributes(
attribute.String("sender", bridgeTx.OriginSender.String()),
))
Expand All @@ -131,75 +132,66 @@
metrics.EndSpanWithErr(span, err)
}()

// TODO: add a cache for decimals.
var originERC20, destERC20 *ierc20.IERC20
res := decimalsRes{}

if bridgeTx.OriginToken == chain.EthAddress {
res.originDecimals = ethDecimals
} else {
// TODO: cleanup duplication, but keep paralellism.
// this is a bit of a pain since it deals w/ 3 different fields, but shouldn't take too long
originClient, err := r.client.GetChainClient(ctx, int(bridgeTx.OriginChainId))
// fetch the token decimals in parallel
g, ctx := errgroup.WithContext(ctx)
g.Go(func() error {
originDecimals, err = r.getDecimals(ctx, bridgeTx.OriginToken, bridgeTx.OriginChainId)

Check warning on line 138 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L136-L138

Added lines #L136 - L138 were not covered by tests
if err != nil {
return nil, fmt.Errorf("could not get origin client: %w", err)
return fmt.Errorf("could not get origin decimals: %w", err)

Check warning on line 140 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L140

Added line #L140 was not covered by tests
}
originERC20, err = ierc20.NewIERC20(bridgeTx.OriginToken, originClient)
return nil

Check warning on line 142 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L142

Added line #L142 was not covered by tests
})
g.Go(func() error {
destDecimals, err = r.getDecimals(ctx, bridgeTx.DestToken, bridgeTx.DestChainId)

Check warning on line 145 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L144-L145

Added lines #L144 - L145 were not covered by tests
if err != nil {
return nil, fmt.Errorf("could not get origin token")
return fmt.Errorf("could not get dest decimals: %w", err)

Check warning on line 147 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L147

Added line #L147 was not covered by tests
}
return nil

Check warning on line 149 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L149

Added line #L149 was not covered by tests
})
err = g.Wait()
if err != nil {
return nil, nil, fmt.Errorf("could not get decimals: %w", err)

Check warning on line 153 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L151-L153

Added lines #L151 - L153 were not covered by tests
Comment on lines +135 to +153
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address potential data race in error handling within goroutines.

- originDecimals, err = r.getDecimals(ctx, bridgeTx.OriginToken, bridgeTx.OriginChainId)
+ originDecimals, localErr := r.getDecimals(ctx, bridgeTx.OriginToken, bridgeTx.OriginChainId)
  if localErr != nil {
-   return fmt.Errorf("could not get origin decimals: %w", err)
+   return fmt.Errorf("could not get origin decimals: %w", localErr)
  }

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// fetch the token decimals in parallel
g, ctx := errgroup.WithContext(ctx)
g.Go(func() error {
originDecimals, err = r.getDecimals(ctx, bridgeTx.OriginToken, bridgeTx.OriginChainId)
if err != nil {
return nil, fmt.Errorf("could not get origin client: %w", err)
return fmt.Errorf("could not get origin decimals: %w", err)
}
originERC20, err = ierc20.NewIERC20(bridgeTx.OriginToken, originClient)
return nil
})
g.Go(func() error {
destDecimals, err = r.getDecimals(ctx, bridgeTx.DestToken, bridgeTx.DestChainId)
if err != nil {
return nil, fmt.Errorf("could not get origin token")
return fmt.Errorf("could not get dest decimals: %w", err)
}
return nil
})
err = g.Wait()
if err != nil {
return nil, nil, fmt.Errorf("could not get decimals: %w", err)
// fetch the token decimals in parallel
g, ctx := errgroup.WithContext(ctx)
g.Go(func() error {
originDecimals, localErr := r.getDecimals(ctx, bridgeTx.OriginToken, bridgeTx.OriginChainId)
if localErr != nil {
return fmt.Errorf("could not get origin decimals: %w", localErr)
}
return nil
})
g.Go(func() error {
destDecimals, err = r.getDecimals(ctx, bridgeTx.DestToken, bridgeTx.DestChainId)
if err != nil {
return fmt.Errorf("could not get dest decimals: %w", err)
}
return nil
})
err = g.Wait()
if err != nil {
return nil, nil, fmt.Errorf("could not get decimals: %w", err)

}

if bridgeTx.DestToken == chain.EthAddress {
res.destDecimals = ethDecimals
} else {
destClient, err := r.client.GetChainClient(ctx, int(bridgeTx.DestChainId))
if err != nil {
return nil, fmt.Errorf("could not get dest client: %w", err)
}
destERC20, err = ierc20.NewIERC20(bridgeTx.DestToken, destClient)
if err != nil {
return nil, fmt.Errorf("could not get dest token")
}
}
return originDecimals, destDecimals, nil

Check warning on line 156 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L156

Added line #L156 was not covered by tests
}

// return early if both dest and origin are ETH.
if originERC20 == nil && destERC20 == nil {
return &res, nil
// getDecimals gets the decimals for a token on a chain.
// It will attempt to load a result from the cache first.
// The cache will be updated if the result is fetched from RPC.
func (r *Relayer) getDecimals(ctx context.Context, addr common.Address, chainID uint32) (decimals *uint8, err error) {
// attempt to load decimal from cache
key := getDecimalsKey(addr, chainID)
decimals, ok := r.decimalsCache.Load(key)
if ok {
return decimals, nil

Check warning on line 167 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L162-L167

Added lines #L162 - L167 were not covered by tests
}

// do rpc calls to fetch the erc20 decimals.
g, ctx := errgroup.WithContext(ctx)
if originERC20 != nil {
g.Go(func() error {
res.originDecimals, err = originERC20.Decimals(&bind.CallOpts{Context: ctx})
if err != nil {
return fmt.Errorf("could not get dest decimals: %w", err)
}
return nil
})
if addr == chain.EthAddress {
return &ethDecimals, nil

Check warning on line 171 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L170-L171

Added lines #L170 - L171 were not covered by tests
}

if destERC20 != nil {
g.Go(func() error {
res.destDecimals, err = destERC20.Decimals(&bind.CallOpts{Context: ctx})
if err != nil {
return fmt.Errorf("could not get origin decimals: %w", err)
}
return nil
})
// fetch decimals from RPC
client, err := r.client.GetChainClient(ctx, int(chainID))
if err != nil {
return nil, fmt.Errorf("could not get client for chain %d: %w", chainID, err)

Check warning on line 177 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L175-L177

Added lines #L175 - L177 were not covered by tests
}

err = g.Wait()
erc20, err := ierc20.NewIERC20(addr, client)
if err != nil {
return nil, fmt.Errorf("could not get token at %s: %w", addr.String(), err)
}
dec, err := erc20.Decimals(&bind.CallOpts{Context: ctx})

Check warning on line 183 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L179-L183

Added lines #L179 - L183 were not covered by tests
if err != nil {
return nil, fmt.Errorf("could not get decimals: %w", err)
}

return &res, nil
// update the cache
r.decimalsCache.Store(key, &dec)
return &dec, nil

Check warning on line 190 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L189-L190

Added lines #L189 - L190 were not covered by tests
}

type decimalsRes struct {
originDecimals, destDecimals uint8
func getDecimalsKey(addr common.Address, chainID uint32) string {
return fmt.Sprintf("%s-%d", addr.Hex(), chainID)

Check warning on line 194 in services/rfq/relayer/service/chainindexer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/chainindexer.go#L193-L194

Added lines #L193 - L194 were not covered by tests
}

func (r *Relayer) handleDepositClaimed(ctx context.Context, event *fastbridge.FastBridgeBridgeDepositClaimed, chainID int) error {
Expand Down
8 changes: 4 additions & 4 deletions services/rfq/relayer/service/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,22 @@
}

// TODO: you can just pull these out of inventory. If they don't exist mark as invalid.
decimals, err := r.getDecimals(ctx, bridgeTx)
originDecimals, destDecimals, err := r.getDecimalsFromBridgeTx(ctx, bridgeTx)

Check warning on line 61 in services/rfq/relayer/service/handlers.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/handlers.go#L61

Added line #L61 was not covered by tests
// can't use errors.is here
if err != nil && strings.Contains(err.Error(), "no contract code at given address") {
logger.Warnf("invalid token, skipping")
return nil
}

if err != nil {
if err != nil || originDecimals == nil || destDecimals == nil {

Check warning on line 68 in services/rfq/relayer/service/handlers.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/handlers.go#L68

Added line #L68 was not covered by tests
return fmt.Errorf("could not get decimals: %w", err)
}

err = r.db.StoreQuoteRequest(ctx, reldb.QuoteRequest{
BlockNumber: req.Raw.BlockNumber,
RawRequest: req.Request,
OriginTokenDecimals: decimals.originDecimals,
DestTokenDecimals: decimals.destDecimals,
OriginTokenDecimals: *originDecimals,
DestTokenDecimals: *destDecimals,

Check warning on line 76 in services/rfq/relayer/service/handlers.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/handlers.go#L75-L76

Added lines #L75 - L76 were not covered by tests
TransactionID: req.TransactionId,
Sender: req.Sender,
Transaction: bridgeTx,
Expand Down
3 changes: 3 additions & 0 deletions services/rfq/relayer/service/relayer.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"github.com/ethereum/go-ethereum/common"
"github.com/ipfs/go-log"
"github.com/jellydator/ttlcache/v3"
"github.com/puzpuzpuz/xsync/v2"
"github.com/synapsecns/sanguine/core/dbcommon"
"github.com/synapsecns/sanguine/core/metrics"
"github.com/synapsecns/sanguine/ethergo/listener"
Expand Down Expand Up @@ -45,6 +46,7 @@
submitter submitter.TransactionSubmitter
signer signer.Signer
claimCache *ttlcache.Cache[common.Hash, bool]
decimalsCache *xsync.MapOf[string, *uint8]
}

var logger = log.Logger("relayer")
Expand Down Expand Up @@ -126,6 +128,7 @@
quoter: q,
metrics: metricHandler,
claimCache: cache,
decimalsCache: xsync.NewMapOf[*uint8](),

Check warning on line 131 in services/rfq/relayer/service/relayer.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/relayer.go#L131

Added line #L131 was not covered by tests
cfg: cfg,
inventory: im,
submitter: sm,
Expand Down
Loading