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 9 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/util"
"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 util.CheckRetryableError(err) {
time.Sleep(7 * time.Second)
continue
}
time.Sleep(7 * time.Second)
return res, err
}
}

Expand Down
1 change: 1 addition & 0 deletions apps/ethereum/account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ func FetchBalanceFromKey(ctx context.Context, rpc, key string) (*big.Int, error)
if err != nil {
return nil, err
}
defer client.Close()

balance, err := client.BalanceAt(ctx, *addr, nil)
if err != nil {
Expand Down
53 changes: 31 additions & 22 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/util"
"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 @@ -146,11 +148,7 @@
if err != nil {
return nil, err
}
blockHeight, err := ethereumNumberToUint64(b.Number)
if err != nil {
return nil, err
}
b.Height = blockHeight
b.Height = uint64(height)
for _, tx := range b.Tx {
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.

RPCGetBlockWithTransactions no longer validates the RPC response’s number field and now unconditionally sets b.Height from the requested height. If the RPC returns null (common when querying beyond the tip) or returns an unexpected block, json.Unmarshal can still succeed with a zero-value struct and this function will incorrectly return a “valid” block with empty hash/number. Consider unmarshalling into a pointer and checking for nil, and/or parsing/validating b.Number (and matching it to height) before returning success.

Copilot uses AI. Check for mistakes.
tx.BlockHash = b.Hash
}
Expand Down Expand Up @@ -284,19 +282,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)
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 👍 / 👎.

if err != nil {
return nil, err
}
Expand Down Expand Up @@ -337,15 +330,31 @@
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 util.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 341 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.
defer client.Close()

for {
logs, err := client.FilterLogs(ctx, query)
if ctx.Err() != nil {
return nil, ctx.Err()
}
if util.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/util"
)

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 util.CheckRetryableError(err) {
time.Sleep(7 * time.Second)
continue
}
time.Sleep(7 * time.Second)
return res, err
}
}

Expand Down
28 changes: 8 additions & 20 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)
if err != nil {
func fetchAssetId(ctx context.Context, mixinId string) string {
asset, err := common.SafeReadAssetUntilSufficient(ctx, mixinId)
if err != nil || asset == 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.
}
3 changes: 1 addition & 2 deletions common/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"strings"

"github.com/MixinNetwork/mixin/crypto"
"github.com/MixinNetwork/safe/mtg"
"github.com/MixinNetwork/safe/util"
"github.com/fox-one/mixin-sdk-go/v2/mixinnet"
)
Expand Down Expand Up @@ -79,7 +78,7 @@ func ExpandTilde(path string) string {
}

func CheckRetryableError(err error) bool {
return mtg.CheckRetryableError(err) || CheckTransactionLockedError(err)
return util.CheckRetryableError(err) || CheckTransactionLockedError(err)
}

func CheckTransactionLockedError(err error) bool {
Expand Down
3 changes: 3 additions & 0 deletions common/wallet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ func (mw *MixinWallet) drainOutputsFromNetwork(ctx context.Context) {
}
members := []string{mw.client.ClientID}
utxos, err := listUnspentUTXOsUntilSufficient(ctx, mw.client, members, 1, "", checkpoint)
if CheckRetryableError(err) {
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.

This retries on CheckRetryableError(err) by immediately continueing without logging the error. Since drainOutputsFromNetwork runs forever, persistent retryable failures would become silent and make it hard to diagnose network/API issues. Consider logging the error (and possibly using a slightly longer backoff on repeated failures) before continuing.

Suggested change
if CheckRetryableError(err) {
if CheckRetryableError(err) {
fmt.Printf("drainOutputsFromNetwork retryable error at checkpoint %d: %v\n", checkpoint, err)

Copilot uses AI. Check for mistakes.
continue
}
if err != nil {
panic(err)
}
Expand Down
54 changes: 15 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 All @@ -18,6 +15,7 @@ import (
"github.com/MixinNetwork/safe/common"
"github.com/MixinNetwork/safe/keeper/store"
"github.com/MixinNetwork/safe/mtg"
"github.com/MixinNetwork/safe/util"
"github.com/gofrs/uuid/v5"
"github.com/shopspring/decimal"
)
Expand Down Expand Up @@ -229,40 +227,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 @@ -276,16 +253,15 @@ func (node *Node) fetchAssetMeta(ctx context.Context, id string) (*store.Asset,
for {
meta, err = node.fetchMixinAsset(ctx, id)
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.
if meta == nil {
return nil, fmt.Errorf("fetchAssetMeta(%s) => nil", id)
}
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 util.CheckRetryableError(err) {
time.Sleep(2 * time.Second)
continue
}
time.Sleep(2 * time.Second)
return nil, err
}
}
4 changes: 2 additions & 2 deletions messenger/mixin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/MixinNetwork/bot-api-go-client/v3"
"github.com/MixinNetwork/mixin/logger"
"github.com/MixinNetwork/safe/common"
"github.com/MixinNetwork/safe/mtg"
"github.com/MixinNetwork/safe/util"
"github.com/fox-one/mixin-sdk-go/v2"
"github.com/gofrs/uuid/v5"
)
Expand Down Expand Up @@ -216,7 +216,7 @@ func (mm *MixinMessenger) SyncAck() bool {
func (mm *MixinMessenger) sendMessagesWithoutTimeout(ctx context.Context, batch []*mixin.MessageRequest) error {
for {
err := mm.client.SendMessages(ctx, batch)
if err != nil && mtg.CheckRetryableError(err) {
if err != nil && util.CheckRetryableError(err) {
logger.Printf("messenger.sendMessagesWithoutTimeout(retry, %d) => %v", len(batch), err)
time.Sleep(3 * time.Second)
continue
Expand Down
3 changes: 2 additions & 1 deletion mtg/deposit.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/mixin/logger"
"github.com/MixinNetwork/safe/util"
)

type DepositEntry struct {
Expand Down Expand Up @@ -61,7 +62,7 @@ func (grp *Group) readOutputDepositUntilSufficientImpl(ctx context.Context, id s
var deposit *SafeDepositView
err := grp.mixin.Get(ctx, fmt.Sprintf("/safe/outputs/%s/deposit", id), nil, &deposit)
logger.Verbosef("Group.readOutputDeposit(%s) => %v %v\n", id, deposit, err)
if CheckRetryableError(err) {
if util.CheckRetryableError(err) {
time.Sleep(3 * time.Second)
continue
}
Expand Down
4 changes: 2 additions & 2 deletions mtg/transaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ func (grp *Group) createMultisigUntilSufficient(ctx context.Context, id, raw str
RawTransaction: raw,
})
logger.Verbosef("Group.SafeCreateTransactionRequest(%s, %s) => %v %v\n", id, raw, req, err)
if err != nil && CheckRetryableError(err) {
if err != nil && util.CheckRetryableError(err) {
time.Sleep(3 * time.Second)
continue
}
Expand Down Expand Up @@ -558,7 +558,7 @@ func (grp *Group) signMultisigUntilSufficient(ctx context.Context, input *mixin.
RawTransaction: signedRaw,
})
logger.Verbosef("Group.SafeSignMultisigRequest(%s %s) => %v %v\n", input.RequestID, signedRaw, req, err)
if err != nil && CheckRetryableError(err) {
if err != nil && util.CheckRetryableError(err) {
time.Sleep(3 * time.Second)
continue
}
Expand Down
Loading
Loading