Skip to content

Improves/request retry #131

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.

Sign up for GitHub

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

Open
hundredark wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Improves/request retry #131

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6c261dd
improve retry
hundredark Feb 27, 2026
b88f342
slight fix
hundredark Feb 27, 2026
49aa2b6
improve ethereum filter logs
hundredark Feb 27, 2026
a3a8d67
slight improve
hundredark Feb 28, 2026
20f3480
slight improve
hundredark Apr 22, 2026
4267bf6
fix issues
hundredark Apr 22, 2026
e229644
slight improve
hundredark Apr 22, 2026
1f6256e
slight fix
hundredark Apr 22, 2026
1ebbe47
slight improves
hundredark Apr 22, 2026
3143bb6
fix issues
hundredark Apr 27, 2026
84724cc
Merge branch 'main' into improves/request-retry
hundredark May 25, 2026
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
14 changes: 5 additions & 9 deletions apps/bitcoin/rpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import (
"math/big"
"net/http"
"sort"
"strings"
"time"

"github.com/MixinNetwork/mixin/logger"
"github.com/MixinNetwork/safe/mtg"
"github.com/btcsuite/btcd/btcutil"
"github.com/btcsuite/btcd/txscript"
"github.com/shopspring/decimal"
Expand Down Expand Up @@ -367,15 +367,11 @@ func callBitcoinRPCUntilSufficient(rpc, method string, params []any) ([]byte, er
return res, nil
}
logger.Printf("callBitcoinRPC(%s, %s, %v) => %v", rpc, method, params, err)
reason := strings.ToLower(err.Error())
switch {
case strings.Contains(reason, "timeout"):
case strings.Contains(reason, "eof"):
case strings.Contains(reason, "handshake"):
default:
return res, err
if mtg.CheckRetryableError(err) {
time.Sleep(7 * time.Second)
continue
}
time.Sleep(7 * time.Second)
return res, err
}
}

Expand Down
45 changes: 25 additions & 20 deletions apps/ethereum/rpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@

"github.com/MixinNetwork/mixin/logger"
"github.com/MixinNetwork/safe/apps/ethereum/abi"
"github.com/MixinNetwork/safe/mtg"
"github.com/ethereum/go-ethereum"
ga "github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/ethclient"
)
Expand Down Expand Up @@ -284,22 +286,14 @@
}

func GetERC20TransferLogFromBlock(ctx context.Context, rpc string, chain, height int64) ([]*Transfer, error) {
client, err := ethclient.Dial(rpc)
if err != nil {
return nil, err
}
query := ethereum.FilterQuery{
FromBlock: big.NewInt(height),
ToBlock: big.NewInt(height),
}
contractAbi, err := ga.JSON(strings.NewReader(abi.AssetABI))
if err != nil {
log.Fatal(err)
}
logs, err := client.FilterLogs(ctx, query)
if err != nil {
return nil, err
}
logs, err := filterLogsUntilSufficient(ctx, rpc, ethereum.FilterQuery{
FromBlock: big.NewInt(height),
ToBlock: big.NewInt(height),
})
Comment on lines +293 to +296
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 6, 2026

Choose a reason for hiding this comment

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

P1 Badge Return filter-log errors before parsing transfers

GetERC20TransferLogFromBlock now calls filterLogsUntilSufficient but never checks the returned err, so any non-retryable FilterLogs failure is silently treated as success and the function returns an empty transfer list. This can hide RPC/query failures (for example invalid filter params or provider-side errors) and cause missed ERC-20 transfer processing instead of surfacing an error to callers.

Useful? React with 👍 / 👎.

logTransferSig := []byte("Transfer(address,address,uint256)")
logTransferSigHash := crypto.Keccak256Hash(logTransferSig)

Expand Down Expand Up @@ -337,15 +331,26 @@
return res, nil
}
logger.Printf("callEthereumRPC(%s, %s, %v) => %v", rpc, method, params, err)
reason := strings.ToLower(err.Error())
switch {
case strings.Contains(reason, "timeout"):
case strings.Contains(reason, "eof"):
case strings.Contains(reason, "handshake"):
default:
return res, err
if mtg.CheckRetryableError(err) {
time.Sleep(7 * time.Second)
continue
}
return res, err
}
}

func filterLogsUntilSufficient(ctx context.Context, rpc string, query ethereum.FilterQuery) ([]types.Log, error) {

Check failure on line 342 in apps/ethereum/rpc.go

View workflow job for this annotation

GitHub Actions / lint 

undefined: ethereum (typecheck)
client, err := ethclient.Dial(rpc)
if err != nil {
return nil, err
}
Comment on lines +345 to +349
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

This dials an ethclient but never closes it; over time this can leak TCP connections/file descriptors (especially if called frequently). Ensure the client is closed (or reuse a shared client) and also consider breaking out when ctx is done to avoid retrying forever on context-related failures.

Copilot uses AI. Check for mistakes.
for {
logs, err := client.FilterLogs(ctx, query)
if mtg.CheckRetryableError(err) {
time.Sleep(7 * time.Second)
continue
}
time.Sleep(7 * time.Second)
return logs, err
Comment on lines +352 to +361
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

If client.FilterLogs returns a retryable error caused by the passed-in ctx (e.g., deadline exceeded), this loop will retry indefinitely with the same expired context. Add an explicit if ctx.Err() != nil { return nil, ctx.Err() } (or avoid treating context deadline errors as retryable) before sleeping/retrying.

Copilot uses AI. Check for mistakes.
}
}

Expand Down
15 changes: 5 additions & 10 deletions apps/mixin/rpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (
"fmt"
"io"
"net/http"
"strings"
"time"

"github.com/MixinNetwork/mixin/logger"
"github.com/MixinNetwork/safe/mtg"
)

type WithdrawalData struct {
Expand Down Expand Up @@ -69,16 +69,11 @@ func callMixinRPCUntilSufficient(rpc, method string, params []any) ([]byte, erro
return res, nil
}
logger.Printf("callMixinRPC(%s, %s, %v) => %v", rpc, method, params, err)
reason := strings.ToLower(err.Error())
switch {
case strings.Contains(reason, "timeout"):
case strings.Contains(reason, "eof"):
case strings.Contains(reason, "handshake"):
case strings.Contains(reason, "invalid character"):
default:
return res, err
if mtg.CheckRetryableError(err) {
time.Sleep(7 * time.Second)
continue
}
time.Sleep(7 * time.Second)
return res, err
}
}

Expand Down
26 changes: 7 additions & 19 deletions cmd/transaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package cmd

import (
"context"
"encoding/base64"
"encoding/hex"
"encoding/json"
"fmt"
"net/http"
"strings"
"time"

"github.com/MixinNetwork/safe/apps/bitcoin"
"github.com/MixinNetwork/safe/apps/ethereum"
Expand All @@ -22,6 +20,7 @@ import (
)

func GenerateTestTransactionProposal(c *cli.Context) error {
ctx := context.Background()
chain := c.Int("chain")
switch chain {
case common.SafeChainBitcoin:
Expand All @@ -43,7 +42,7 @@ func GenerateTestTransactionProposal(c *cli.Context) error {

addr := abi.GetFactoryAssetAddress("0x11EC02748116A983deeD59235302C3139D6e8cdD", common.SafeBitcoinChainId, "BTC", "Bitcoin", holder)
assetKey := strings.ToLower(addr.String())
bondId := fetchAssetId(ethereum.GenerateAssetId(common.SafeChainPolygon, assetKey))
bondId := fetchAssetId(ctx, ethereum.GenerateAssetId(common.SafeChainPolygon, assetKey))

extra := []byte(receiver)
sid := uuid.Must(uuid.NewV4()).String()
Expand Down Expand Up @@ -94,24 +93,13 @@ func GenerateTestTransactionApproval(c *cli.Context) error {
return nil
}

func fetchAssetId(mixinId string) string {
client := &http.Client{Timeout: 10 * time.Second}
path := "https://api.mixin.one/network/assets/" + mixinId
resp, err := client.Get(path)
func fetchAssetId(ctx context.Context, mixinId string) string {
asset, err := common.SafeReadAssetUntilSufficient(ctx, mixinId)
if err != nil {
panic(mixinId)
}
defer resp.Body.Close()

var body struct {
Data struct {
AssetId string `json:"asset_id"`
MixinId string `json:"mixin_id"`
} `json:"data"`
}
json.NewDecoder(resp.Body).Decode(&body)
if body.Data.MixinId != mixinId {
if asset.KernelAssetID != mixinId {
panic(mixinId)
}
return body.Data.AssetId
return asset.AssetID
Comment on lines +97 to +104
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

SafeReadAssetUntilSufficient returns (nil, nil) on 404 (see common/mixin.go), so asset can be nil here; dereferencing asset.KernelAssetID / asset.AssetID will panic. Handle the asset == nil case explicitly (e.g., panic with a clearer message or return an empty string / error depending on expected CLI behavior).

Copilot uses AI. Check for mistakes.
}
4 changes: 4 additions & 0 deletions common/wallet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/MixinNetwork/bot-api-go-client/v3"
"github.com/MixinNetwork/safe/mtg"
"github.com/fox-one/mixin-sdk-go/v2"
"github.com/fox-one/mixin-sdk-go/v2/mixinnet"
"github.com/shopspring/decimal"
Expand Down Expand Up @@ -36,6 +37,9 @@ func (mw *MixinWallet) drainOutputsFromNetwork(ctx context.Context) {
}
members := []string{mw.client.ClientID}
utxos, err := listUnspentUTXOsUntilSufficient(ctx, mw.client, members, 1, "", checkpoint)
if mtg.CheckRetryableError(err) {
continue
}
if err != nil {
panic(err)
}
Expand Down
50 changes: 11 additions & 39 deletions keeper/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@ import (
"context"
"encoding/binary"
"encoding/hex"
"encoding/json"
"fmt"
"math/big"
"net/http"
"strings"
"time"

"github.com/MixinNetwork/mixin/crypto"
Expand Down Expand Up @@ -229,40 +226,19 @@ func (node *Node) fetchAssetMetaFromMessengerOrEthereum(ctx context.Context, id,
return asset, node.store.WriteAssetMeta(ctx, asset)
}

func (node *Node) fetchMixinAsset(_ context.Context, id string) (*store.Asset, error) {
client := &http.Client{Timeout: 10 * time.Second}
path := node.conf.MixinMessengerAPI + "/network/assets/" + id
resp, err := client.Get(path)
if err != nil {
return nil, err
}
defer common.CloseOrPanic(resp.Body)

var body struct {
Data *struct {
AssetId string `json:"asset_id"`
MixinId crypto.Hash `json:"mixin_id"`
AssetKey string `json:"asset_key"`
Symbol string `json:"symbol"`
Name string `json:"name"`
Precision uint32 `json:"precision"`
ChainId string `json:"chain_id"`
} `json:"data"`
}
err = json.NewDecoder(resp.Body).Decode(&body)
if err != nil {
func (node *Node) fetchMixinAsset(ctx context.Context, id string) (*store.Asset, error) {
asset, err := common.SafeReadAssetUntilSufficient(ctx, id)
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 6, 2026

Choose a reason for hiding this comment

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

P2 Badge Preserve messenger API config in asset metadata fetch

This change removes use of node.conf.MixinMessengerAPI from keeper asset lookups by switching to common.SafeReadAssetUntilSufficient(ctx, id), which takes no API endpoint argument. As a result, deployments that rely on a non-default mixin-messenger-api value will no longer query their configured endpoint here, and asset metadata fetches can fail in those environments.

Useful? React with 👍 / 👎.

if err != nil || asset == nil {
return nil, err
}
asset := body.Data

return &store.Asset{
AssetId: asset.AssetId,
MixinId: asset.MixinId.String(),
AssetId: asset.AssetID,
MixinId: asset.KernelAssetID,
AssetKey: asset.AssetKey,
Symbol: asset.Symbol,
Name: asset.Name,
Decimals: asset.Precision,
Chain: common.SafeAssetIdChain(asset.ChainId),
Decimals: uint32(asset.Precision),
Chain: common.SafeAssetIdChainNoPanic(asset.ChainID),
CreatedAt: time.Now().UTC(),
}, nil
}
Expand All @@ -278,14 +254,10 @@ func (node *Node) fetchAssetMeta(ctx context.Context, id string) (*store.Asset,
if err == nil {
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

fetchMixinAsset can return (nil, nil) when the asset is not found (404), which makes meta nil here; calling WriteAssetMeta(ctx, meta) will panic because it dereferences asset. Add a meta == nil branch (similar to observer/bond.go) to return (nil, nil) without writing.

Suggested change
if err == nil {
if err == nil {
if meta == nil {
return nil, nil
}

Copilot uses AI. Check for mistakes.
return meta, node.store.WriteAssetMeta(ctx, meta)
}
reason := strings.ToLower(err.Error())
switch {
case strings.Contains(reason, "timeout"):
case strings.Contains(reason, "eof"):
case strings.Contains(reason, "handshake"):
default:
return nil, err
if mtg.CheckRetryableError(err) {
time.Sleep(2 * time.Second)
continue
}
time.Sleep(2 * time.Second)
return nil, err
}
}
13 changes: 7 additions & 6 deletions mtg/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,17 @@ func CheckRetryableError(err error) bool {
if err == nil {
return false
}
es := err.Error()
es := strings.ToLower(err.Error())
switch {
case strings.Contains(es, "EOF"):
case strings.Contains(es, "eof"):
case strings.Contains(es, "timeout"):
case strings.Contains(es, "timed out"):
case strings.Contains(es, "handshake"):
case strings.Contains(es, "context deadline exceeded"):
case strings.Contains(es, "connection reset by peer"):
case strings.Contains(es, "Client.Timeout exceeded"):
case strings.Contains(es, "Bad Gateway"):
case strings.Contains(es, "Internal Server Error"):
case strings.Contains(es, "bad gateway"):
case strings.Contains(es, "internal server error"):
case strings.Contains(es, "invalid character '<' looking for beginning of value"):
case strings.Contains(es, "TLS handshake timeout"):
default:
return false
}
Expand Down
42 changes: 12 additions & 30 deletions observer/bond.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ package observer

import (
"context"
"encoding/json"
"fmt"
"net/http"
"strings"
"time"

Expand All @@ -15,6 +13,7 @@ import (
"github.com/MixinNetwork/safe/apps/ethereum"
"github.com/MixinNetwork/safe/common"
"github.com/MixinNetwork/safe/common/abi"
"github.com/MixinNetwork/safe/mtg"
)

type MixinNetworkAsset struct {
Expand Down Expand Up @@ -126,32 +125,19 @@ func (node *Node) fetchAssetMetaFromMessengerOrEthereum(ctx context.Context, id,
return asset, node.store.WriteAssetMeta(ctx, asset)
}

func (node *Node) fetchMixinAsset(_ context.Context, id string) (*Asset, error) {
client := &http.Client{Timeout: 10 * time.Second}
path := node.conf.MixinMessengerAPI + "/network/assets/" + id
resp, err := client.Get(path)
if err != nil {
func (node *Node) fetchMixinAsset(ctx context.Context, id string) (*Asset, error) {
asset, err := common.SafeReadAssetUntilSufficient(ctx, id)
if err != nil || asset == nil {
return nil, err
}
Comment on lines +128 to 132
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

common.SafeReadAssetUntilSufficient returns (asset=nil, err=nil) when the upstream returns 404 (see common/mixin.go). In that case this function returns (nil, nil), which makes it easy for callers to treat "not found" as success and then panic/nil-deref. Consider converting the asset == nil case into an explicit error (e.g. ErrNotFound) so callers can branch reliably.

Copilot uses AI. Check for mistakes.
defer resp.Body.Close()

var body struct {
Data *MixinNetworkAsset `json:"data"`
}
err = json.NewDecoder(resp.Body).Decode(&body)
if err != nil || body.Data == nil {
return nil, err
}
asset := body.Data

return &Asset{
AssetId: asset.AssetId,
MixinId: asset.MixinId.String(),
AssetId: asset.AssetID,
MixinId: asset.KernelAssetID,
AssetKey: asset.AssetKey,
Symbol: asset.Symbol,
Name: asset.Name,
Decimals: asset.Precision,
Chain: common.SafeAssetIdChainNoPanic(asset.ChainId),
Decimals: uint32(asset.Precision),
Chain: common.SafeAssetIdChainNoPanic(asset.ChainID),
CreatedAt: time.Now().UTC(),
}, nil
}
Expand All @@ -173,15 +159,11 @@ func (node *Node) fetchAssetMeta(ctx context.Context, id string) (*Asset, error)
}
return meta, node.store.WriteAssetMeta(ctx, meta)
}
reason := strings.ToLower(err.Error())
switch {
case strings.Contains(reason, "timeout"):
case strings.Contains(reason, "eof"):
case strings.Contains(reason, "handshake"):
default:
return nil, err
if mtg.CheckRetryableError(err) {
time.Sleep(2 * time.Second)
continue
}
time.Sleep(2 * time.Second)
return nil, err
}
}

Expand Down
Loading