Skip to content

Commit 0e4d868

Browse files
fix: stale smart wallet detection, dep CVE bumps, gRPC log noise, CI runners (#520)
Co-authored-by: Wei Lin <wei@avaprotocol.org>
1 parent 1ebc3d3 commit 0e4d868

13 files changed

Lines changed: 1292 additions & 42 deletions

File tree

.github/workflows/claude-code-review.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ jobs:
2121
# github.event.pull_request.user.login == 'new-developer' ||
2222
# github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR')
2323

24-
runs-on: blacksmith-4vcpu-ubuntu-2404
24+
runs-on: ubuntu-latest
2525
permissions:
2626
contents: read
2727
pull-requests: write

.github/workflows/claude.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ jobs:
1717
(github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) ||
1818
(github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) ||
1919
(github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude')))
20-
runs-on: blacksmith-4vcpu-ubuntu-2404
20+
runs-on: ubuntu-latest
2121
permissions:
2222
contents: read
2323
pull-requests: write

.github/workflows/release-on-pr-close.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
if: |
1414
github.event.pull_request.merged == true &&
1515
(github.event.pull_request.base.ref == 'main' || github.event.pull_request.base.ref == 'staging')
16-
runs-on: blacksmith-4vcpu-ubuntu-2404
16+
runs-on: ubuntu-latest
1717
permissions:
1818
contents: write # Needed for go-semantic-release to create tags and releases
1919
packages: write # If you use GitHub Packages for Docker images (good to have)

cmd/backfillWalletSaltIndex.go

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
package cmd
2+
3+
import (
4+
"fmt"
5+
"math/big"
6+
"os"
7+
8+
"github.com/AvaProtocol/EigenLayer-AVS/core/chainio/aa"
9+
avsconfig "github.com/AvaProtocol/EigenLayer-AVS/core/config"
10+
"github.com/AvaProtocol/EigenLayer-AVS/core/taskengine"
11+
"github.com/AvaProtocol/EigenLayer-AVS/storage"
12+
"github.com/ethereum/go-ethereum/common"
13+
"github.com/ethereum/go-ethereum/ethclient"
14+
"github.com/spf13/cobra"
15+
)
16+
17+
var (
18+
backfillWalletSaltIndexDryRun bool
19+
backfillWalletSaltIndexVerbose bool
20+
21+
backfillWalletSaltIndexCmd = &cobra.Command{
22+
Use: "backfill-wallet-salt-index",
23+
Short: "Backfill the (owner, factory, salt) → wallet address secondary index",
24+
Long: `Walks every persisted wallet record and either:
25+
26+
1. Writes a (owner, factory, salt) → address secondary index entry, when
27+
the live derivation against the smart-wallet RPC still matches the
28+
stored address (canonical row); or
29+
2. Marks the row stale (StaleDerivation = true, IsHidden = true) when the
30+
derivation differs — i.e. the factory's account implementation has been
31+
upgraded since the row was first persisted.
32+
33+
This subcommand reads db_path and smart_wallet.eth_rpc_url from the same
34+
aggregator YAML the running aggregator uses, so it must be invoked with
35+
--config pointing at that file.
36+
37+
OPERATIONAL CONSTRAINT: BadgerDB is single-writer. The aggregator must be
38+
stopped before this command runs, otherwise opening the database will fail
39+
with a lock error. Always run with --dry-run first against a fresh backup.
40+
41+
Production usage (Docker):
42+
43+
docker stop aggregator-base
44+
docker run --rm --volumes-from aggregator-base avaprotocol/ap-avs:latest \
45+
backfill-wallet-salt-index --config /app/config/aggregator-base.yaml --dry-run
46+
# inspect summary, then re-run without --dry-run
47+
docker start aggregator-base
48+
`,
49+
Run: func(cmd *cobra.Command, args []string) {
50+
if err := runBackfillWalletSaltIndex(config); err != nil {
51+
fmt.Fprintln(os.Stderr, err)
52+
os.Exit(1)
53+
}
54+
},
55+
}
56+
)
57+
58+
func init() {
59+
backfillWalletSaltIndexCmd.Flags().BoolVar(&backfillWalletSaltIndexDryRun, "dry-run", false, "Print what would change without writing to BadgerDB")
60+
backfillWalletSaltIndexCmd.Flags().BoolVar(&backfillWalletSaltIndexVerbose, "verbose", false, "Print one line per wallet processed")
61+
rootCmd.AddCommand(backfillWalletSaltIndexCmd)
62+
}
63+
64+
func runBackfillWalletSaltIndex(configPath string) error {
65+
nodeConfig, err := avsconfig.NewConfig(configPath)
66+
if err != nil {
67+
return fmt.Errorf("parse config %s: %w", configPath, err)
68+
}
69+
if nodeConfig.DbPath == "" {
70+
return fmt.Errorf("config %s has no db_path", configPath)
71+
}
72+
if nodeConfig.SmartWallet == nil || nodeConfig.SmartWallet.EthRpcUrl == "" {
73+
return fmt.Errorf("config %s has no smart_wallet.eth_rpc_url", configPath)
74+
}
75+
76+
fmt.Printf("Config: %s\n", configPath)
77+
fmt.Printf("DB path: %s\n", nodeConfig.DbPath)
78+
// Redact: RPC URLs from Tenderly/Alchemy/Infura embed API keys in
79+
// the path or query — printing the raw URL leaks secrets to terminal
80+
// scrollback and CI logs.
81+
fmt.Printf("RPC URL: %s\n", taskengine.RedactRPCURL(nodeConfig.SmartWallet.EthRpcUrl))
82+
fmt.Printf("Mode: %s\n", backfillModeLabel(backfillWalletSaltIndexDryRun))
83+
fmt.Println()
84+
85+
rpcClient, err := ethclient.Dial(nodeConfig.SmartWallet.EthRpcUrl)
86+
if err != nil {
87+
return fmt.Errorf("dial smart-wallet RPC %s: %w", nodeConfig.SmartWallet.EthRpcUrl, err)
88+
}
89+
defer rpcClient.Close()
90+
91+
db, err := storage.NewWithPath(nodeConfig.DbPath)
92+
if err != nil {
93+
return fmt.Errorf("open BadgerDB at %s: %w\n\nIs the aggregator still running? It must be stopped first.", nodeConfig.DbPath, err)
94+
}
95+
defer db.Close()
96+
97+
deriver := func(owner common.Address, factory common.Address, salt *big.Int) (common.Address, error) {
98+
addr, derr := aa.GetSenderAddressForFactory(rpcClient, owner, factory, salt)
99+
if derr != nil {
100+
return common.Address{}, derr
101+
}
102+
if addr == nil {
103+
return common.Address{}, fmt.Errorf("derived nil address")
104+
}
105+
return *addr, nil
106+
}
107+
108+
stats, err := taskengine.BackfillWalletSaltIndex(db, deriver, taskengine.WalletSaltIndexBackfillOptions{
109+
DryRun: backfillWalletSaltIndexDryRun,
110+
Verbose: backfillWalletSaltIndexVerbose,
111+
Logf: func(format string, args ...any) {
112+
fmt.Printf(format+"\n", args...)
113+
},
114+
})
115+
if err != nil {
116+
return fmt.Errorf("backfill: %w", err)
117+
}
118+
119+
fmt.Println()
120+
fmt.Println("Summary")
121+
fmt.Println("-------")
122+
fmt.Printf("Total wallet rows scanned: %d\n", stats.Total)
123+
fmt.Printf(" Canonical (live derive matches): %d\n", stats.CanonicalConfirmed)
124+
fmt.Printf(" Secondary index newly written: %d\n", stats.SecondaryIndexWritten)
125+
fmt.Printf(" Secondary index already correct: %d\n", stats.SecondaryIndexExisting)
126+
fmt.Printf(" Stale (live derive differs): %d\n", stats.NewlyMarkedStale+stats.AlreadyStale)
127+
fmt.Printf(" Newly marked stale: %d\n", stats.NewlyMarkedStale)
128+
fmt.Printf(" Already marked stale: %d\n", stats.AlreadyStale)
129+
fmt.Printf(" Skipped — missing factory: %d\n", stats.SkippedMissingFactory)
130+
fmt.Printf(" Skipped — missing salt: %d\n", stats.SkippedMissingSalt)
131+
fmt.Printf(" Skipped — RPC derive error: %d\n", stats.SkippedDeriveError)
132+
if backfillWalletSaltIndexDryRun {
133+
fmt.Println()
134+
fmt.Println("DRY RUN — no changes written. Re-run without --dry-run to apply.")
135+
}
136+
return nil
137+
}
138+
139+
func backfillModeLabel(dryRun bool) string {
140+
if dryRun {
141+
return "DRY RUN"
142+
}
143+
return "APPLY"
144+
}

core/taskengine/engine.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,10 +465,43 @@ func (n *Engine) MustStart() error {
465465
return nil
466466
}
467467

468+
// markPreviousCanonicalStaleIfAny consults the (owner, factory, salt)
469+
// secondary index. If the index already points at a *different* wallet
470+
// address than the freshly derived one, that means the factory's account
471+
// implementation was upgraded between the previous record and now — the
472+
// previously canonical wallet is no longer derivable from this triple.
473+
// Mark the old record as stale so list responses hide it and future
474+
// writes can replace the index entry. Index lookup or marking failures
475+
// are logged but never block the fresh insertion path.
476+
func (n *Engine) markPreviousCanonicalStaleIfAny(owner common.Address, factoryAddr common.Address, salt *big.Int, freshAddress common.Address) {
477+
previousAddr, err := LookupCanonicalWalletAddress(n.db, owner, factoryAddr, salt)
478+
if err == badger.ErrKeyNotFound {
479+
return
480+
}
481+
if err != nil {
482+
n.logger.Warn("Failed to look up canonical wallet address by (owner, factory, salt)", "owner", owner.Hex(), "factory", factoryAddr.Hex(), "salt", salt.String(), "error", err)
483+
return
484+
}
485+
if strings.EqualFold(previousAddr.Hex(), freshAddress.Hex()) {
486+
return
487+
}
488+
n.logger.Error("Stale wallet derivation detected — factory implementation likely upgraded",
489+
"owner", owner.Hex(),
490+
"factory", factoryAddr.Hex(),
491+
"salt", salt.String(),
492+
"previousAddress", previousAddr.Hex(),
493+
"newAddress", freshAddress.Hex(),
494+
)
495+
if markErr := MarkWalletStale(n.db, owner, previousAddr.Hex()); markErr != nil {
496+
n.logger.Warn("Failed to mark previous canonical wallet as stale", "owner", owner.Hex(), "previousAddress", previousAddr.Hex(), "error", markErr)
497+
}
498+
}
499+
468500
// storeDefaultWalletForListWallets creates and stores the default wallet (salt:0) for ListWallets.
469501
// Returns the stored wallet model if successful, or nil if storage failed (but logs the error).
470502
func (n *Engine) storeDefaultWalletForListWallets(owner common.Address, defaultDerivedAddress *common.Address, defaultSystemFactory common.Address, defaultSalt *big.Int) *model.SmartWallet {
471503
n.logger.Info("Default wallet not found in DB for ListWallets, storing it", "owner", owner.Hex(), "walletAddress", defaultDerivedAddress.Hex())
504+
n.markPreviousCanonicalStaleIfAny(owner, defaultSystemFactory, defaultSalt, *defaultDerivedAddress)
472505
newModelWallet := &model.SmartWallet{
473506
Owner: &owner,
474507
Address: defaultDerivedAddress,
@@ -606,6 +639,15 @@ func (n *Engine) ListWallets(owner common.Address, payload *avsproto.ListWalletR
606639
continue
607640
}
608641

642+
// Hard-filter zombies left over from a factory account-implementation
643+
// upgrade. The record is preserved on disk (the on-chain wallet may
644+
// still hold assets), but it is no longer derivable from its
645+
// (owner, factory, salt) and would otherwise show up as a phantom
646+
// salt collision in the response.
647+
if storedModelWallet.StaleDerivation {
648+
continue
649+
}
650+
609651
if processedAddresses[strings.ToLower(storedModelWallet.Address.Hex())] {
610652
continue
611653
}
@@ -738,6 +780,7 @@ func (n *Engine) GetWallet(user *model.User, payload *avsproto.GetWalletReq) (*a
738780
}
739781

740782
n.logger.Info("Wallet not found in DB for GetWallet, creating new entry", "owner", user.Address.Hex(), "walletAddress", derivedSenderAddress.Hex())
783+
n.markPreviousCanonicalStaleIfAny(user.Address, factoryAddr, saltBig, *derivedSenderAddress)
741784
newModelWallet := &model.SmartWallet{
742785
Owner: &user.Address,
743786
Address: derivedSenderAddress,

0 commit comments

Comments
 (0)