Skip to content

Commit ae00ab8

Browse files
committed
fix(tools/talis): finalize fibre setup race fixes
Three follow-up bugs surfaced from the PR #3303 follow-up verification run on a 3-validator AWS Fibre cluster: - aws.go: CreateAWSInstances exited 0 even when individual instance launches failed, so `talis up` lied about success and downstream steps proceeded against a partial cluster. Returns a joined error now so failure cascades stop early. - download.go: sshExec used cmd.CombinedOutput, mixing SSH warnings (the "Warning: Permanently added '...'..." chatter on stderr) into bytes the caller hands to fmt.Sscanf("%d"). The CLI-side providers cross-check parsed those warnings as 0 and looped until its 5-min deadline even though a direct SSH query showed all 3 providers registered. Switch to cmd.Output() (stdout only) and add `-q -o LogLevel=ERROR` to silence the chatter for any caller that does combine streams. - fibre_setup.go: the per-validator escrow verification used `celestia-appd query fibre escrow` which doesn't exist — the actual subcommand is `escrow-account`. The query errored on every retry, the grep for "amount" never matched, and the script wedged on the 3-min deadline reporting `FATAL: fibre-0 escrow not present`. Switch to `escrow-account` and key on `"found":true` (the explicit existence flag in the response). Also wrap the fibre-0 deposit-to-escrow itself in a retry loop matching set-host — same `--yes`-returns-before-inclusion silent-failure mode bit it. fibre-1..N stay best-effort.
1 parent 26f6260 commit ae00ab8

3 files changed

Lines changed: 77 additions & 46 deletions

File tree

tools/talis/aws.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,16 +350,28 @@ func CreateAWSInstances(ctx context.Context, insts []Instance, sshKey, keyName s
350350
close(results)
351351
}()
352352

353-
var created []Instance
353+
var (
354+
created []Instance
355+
failures []string
356+
)
354357
for res := range results {
355358
if res.err != nil {
356359
fmt.Printf("❌ %s failed after %v %v\n", res.inst.Name, res.timeRequired, res.err)
360+
failures = append(failures, fmt.Sprintf("%s: %v", res.inst.Name, res.err))
357361
} else {
358362
created = append(created, res.inst)
359363
fmt.Printf("✅ %s is up (public=%s) in %v\n", res.inst.Name, res.inst.PublicIP, res.timeRequired)
360364
}
361365
fmt.Printf("---- Progress: %d/%d\n", len(created), total)
362366
}
367+
if len(failures) > 0 {
368+
// Surface partial-failure as an error so `talis up` exits
369+
// non-zero; without this, downstream genesis runs against a
370+
// half-provisioned config and fails much later with confusing
371+
// "X has no public IP yet" messages.
372+
return created, fmt.Errorf("%d/%d instance(s) failed to launch: %s",
373+
len(failures), total, strings.Join(failures, "; "))
374+
}
363375
return created, nil
364376
}
365377

tools/talis/download.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,16 +193,26 @@ func compressAndDownload(table, localPath, user, host, sshKeyPath string) error
193193
return nil
194194
}
195195

196-
// sshExec runs a command on a remote host via SSH and returns the combined output.
196+
// sshExec runs a command on a remote host via SSH and returns stdout only.
197+
//
198+
// We intentionally do NOT use CombinedOutput here. ssh prints connection
199+
// chatter ("Warning: Permanently added '...' to the list of known hosts.")
200+
// on stderr, and a previous `CombinedOutput` revision caused
201+
// `fmt.Sscanf(out, "%d")` parses to silently return 0 because the leading
202+
// stderr line had no digits. Capturing only stdout keeps numeric output
203+
// parseable; -q + LogLevel=ERROR further suppresses the chatter for any
204+
// caller that does combine streams.
197205
func sshExec(user, host, sshKeyPath, command string) ([]byte, error) {
198206
cmd := exec.Command("ssh",
207+
"-q",
208+
"-o", "LogLevel=ERROR",
199209
"-o", "StrictHostKeyChecking=no",
200210
"-o", "UserKnownHostsFile=/dev/null",
201211
"-i", sshKeyPath,
202212
fmt.Sprintf("%s@%s", user, host),
203213
command,
204214
)
205-
return cmd.CombinedOutput()
215+
return cmd.Output()
206216
}
207217

208218
func sftpDownload(remotePath, localPath, user, host, sshKeyPath string) error {

tools/talis/fibre_setup.go

Lines changed: 52 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -58,25 +58,24 @@ func setupFibreCmd() *cobra.Command {
5858
// the CLI level (`--yes` returns the txhash before block
5959
// inclusion), but the tx never lands. Polling explicitly
6060
// avoids the `sleep 10` heuristic that used to be here.
61-
sb.WriteString(fmt.Sprintf(
62-
"echo 'waiting for chain to produce first block...'\n"+
63-
"DEADLINE=$(( $(date +%%s) + 300 ))\n"+
64-
"while true; do\n"+
65-
" H=$(celestia-appd status --chain-id %s 2>/dev/null | "+
66-
" grep -oE '\"latest_block_height\":\"[0-9]+\"' | "+
67-
" grep -oE '[0-9]+' | head -1)\n"+
68-
" if [ -n \"$H\" ] && [ \"$H\" -gt 0 ]; then\n"+
69-
" echo \"chain is at height $H\"\n"+
70-
" break\n"+
71-
" fi\n"+
72-
" if [ $(date +%%s) -gt $DEADLINE ]; then\n"+
73-
" echo 'FATAL: chain never produced a block within 5m' >&2\n"+
74-
" exit 1\n"+
75-
" fi\n"+
76-
" sleep 3\n"+
61+
sb.WriteString(
62+
"echo 'waiting for chain to produce first block...'\n" +
63+
"DEADLINE=$(( $(date +%s) + 300 ))\n" +
64+
"while true; do\n" +
65+
" H=$(celestia-appd status 2>/dev/null | " +
66+
" grep -oE '\"latest_block_height\":\"[0-9]+\"' | " +
67+
" grep -oE '[0-9]+' | head -1)\n" +
68+
" if [ -n \"$H\" ] && [ \"$H\" -gt 0 ]; then\n" +
69+
" echo \"chain is at height $H\"\n" +
70+
" break\n" +
71+
" fi\n" +
72+
" if [ $(date +%s) -gt $DEADLINE ]; then\n" +
73+
" echo 'FATAL: chain never produced a block within 5m' >&2\n" +
74+
" exit 1\n" +
75+
" fi\n" +
76+
" sleep 3\n" +
7777
"done\n",
78-
cfg.ChainID,
79-
))
78+
)
8079

8180
// 1. Register fibre host address. Plain `host:port` form —
8281
// x/valaddr requires it; the gRPC client dials it via the
@@ -113,44 +112,54 @@ func setupFibreCmd() *cobra.Command {
113112
cfg.ChainID,
114113
))
115114

116-
// 2. Deposit escrow for each fibre worker account
117-
for i := range fibreAccounts {
118-
keyName := fmt.Sprintf("fibre-%d", i)
119-
sb.WriteString(fmt.Sprintf(
120-
"celestia-appd tx fibre deposit-to-escrow %s "+
121-
"--from %s --keyring-backend=test --home .celestia-app "+
122-
"--chain-id %s --fees %s --yes\n",
123-
escrowAmount,
124-
keyName,
125-
cfg.ChainID, fees,
126-
))
127-
}
128-
129-
// 3. Verify the FIRST fibre account's escrow actually
130-
// landed before we let the tmux session exit. If even
131-
// fibre-0 isn't funded, every Fibre upload from the
132-
// runner fails with `escrow account not found for
133-
// signer …` — same silent-failure mode as set-host.
134-
// Other accounts (fibre-1..N) are funded best-effort:
135-
// the runner only signs with fibre-0 by default.
115+
// 2. Deposit escrow for fibre-0 inside a retry loop.
116+
// Same silent-failure mode as set-host: `--yes` returns
117+
// the txhash before inclusion, so a single bounced tx
118+
// (mempool full, signer not yet propagated, …) leaves
119+
// the runner failing every upload with
120+
// `escrow account not found for signer …`. fibre-0 is
121+
// the one the runner actually signs with by default,
122+
// so it's the only one we hard-block on.
136123
sb.WriteString(fmt.Sprintf(
137124
"FIBRE0_ADDR=$(celestia-appd keys show fibre-0 --keyring-backend test --home .celestia-app -a)\n"+
138-
"DEADLINE=$(( $(date +%%s) + 180 ))\n"+
125+
"DEADLINE=$(( $(date +%%s) + 300 ))\n"+
139126
"while true; do\n"+
140-
" if celestia-appd query fibre escrow \"$FIBRE0_ADDR\" --chain-id %s -o json 2>/dev/null \\\n"+
141-
" | grep -q '\"amount\"'; then\n"+
127+
" celestia-appd tx fibre deposit-to-escrow %s "+
128+
"--from fibre-0 --keyring-backend=test --home .celestia-app "+
129+
"--chain-id %s --fees %s --yes >/dev/null 2>&1 || true\n"+
130+
" sleep 6\n"+
131+
" if celestia-appd query fibre escrow-account \"$FIBRE0_ADDR\" --chain-id %s -o json 2>/dev/null \\\n"+
132+
" | grep -q '\"found\":true'; then\n"+
142133
" echo \"escrow confirmed for fibre-0 ($FIBRE0_ADDR)\"\n"+
143134
" break\n"+
144135
" fi\n"+
145136
" if [ $(date +%%s) -gt $DEADLINE ]; then\n"+
146-
" echo \"FATAL: fibre-0 escrow not present after 3m\" >&2\n"+
137+
" echo \"FATAL: fibre-0 escrow did not land after 5m\" >&2\n"+
147138
" exit 1\n"+
148139
" fi\n"+
149-
" sleep 5\n"+
140+
" echo 'fibre-0 escrow pending, retrying...'\n"+
150141
"done\n",
142+
escrowAmount,
143+
cfg.ChainID, fees,
151144
cfg.ChainID,
152145
))
153146

147+
// 3. Best-effort fund fibre-1..N. The runner only signs
148+
// with fibre-0 by default, so a missing one of these
149+
// doesn't block uploads — they exist as headroom for
150+
// future signer rotation.
151+
for i := 1; i < fibreAccounts; i++ {
152+
keyName := fmt.Sprintf("fibre-%d", i)
153+
sb.WriteString(fmt.Sprintf(
154+
"celestia-appd tx fibre deposit-to-escrow %s "+
155+
"--from %s --keyring-backend=test --home .celestia-app "+
156+
"--chain-id %s --fees %s --yes\n",
157+
escrowAmount,
158+
keyName,
159+
cfg.ChainID, fees,
160+
))
161+
}
162+
154163
script := sb.String()
155164

156165
sem <- struct{}{}

0 commit comments

Comments
 (0)