Skip to content

fix(upgrade-test): gate proposal on each validator's local height; retry signer account NotFound - #586

Merged
bdchatham merged 2 commits into
mainfrom
devin/1790867484-harden-upgrade-propose
Oct 1, 2026
Merged

bdchatham merged 2 commits into
mainfrom
devin/1790867484-harden-upgrade-propose

Conversation

@bdchatham

Copy link
Copy Markdown
Collaborator

Summary

Hardens TestNightlyChainUpgrade against the startup race that failed Harbor nightly run nightly-harness-suite-29847360 (PagerDuty Q01DQFA79YE621).

What happened: the test submitted the gov proposal as soon as the aggregate RPC showed height ≥1. Validators 1–3 make up a 3-of-4 quorum, so the aggregate was already at height 7 while validator-0, which joined consensus late, had no committed state yet. The GovSoftwareUpgrade task signs against validator-0's own seid, and that node returned account retrieve sei1…: rpc error: code = NotFound … key not found for its own genesis account.

  • test (upgrade_test.go): replaces the aggregate pollHeightAtLeast(tmRPC, 1) with awaitAllValidatorsAtHeight(..., "genesis-await", 1, ...), which runs one AwaitNodesAtHeight task per validator so the gate checks each validator's own height. The helper takes a new step argument so it can run twice per chain without task-name collisions. The post-upgrade gate keeps the "await" prefix, so its task names are unchanged.
  • sidecar (sign_and_broadcast.go): a new function, retrieveAccount, re-polls AccountNumberSequence while it returns gRPC codes.NotFound, every accountNotFoundPollInterval (1s) for up to accountNotFoundTimeout (60s). Callers can still cancel it through ctx. Other errors return immediately. If the timeout runs out, the original NotFound error is returned, still non-Terminal, so the task-level retry still applies. Broadcast never happens without a resolved account.
  • sidecar/go.mod: google.golang.org/grpc is now a direct dependency (it was indirect before; the version is unchanged).

After this merges, a follow-up platform PR will bump the integration-harness image in clusters/harbor/nightly/harness/cronjobs.yaml.

Verified locally: make test, make tidy-check, golangci-lint --new-from-merge-base=origin/main (sidecar/tasks, and test/integration with -tags integration), and go vet -tags integration ./test/integration/. The nightly upgrade suite itself was not run.

Link to Devin session: https://app.devin.ai/sessions/a3d234c649fe4c17900ce1807a32b433
Open in Devin Desktop: https://app.devin.ai/desktop/session/a3d234c649fe4c17900ce1807a32b433?variant=devin
Requested by: @bdchatham

…try signer account NotFound

The nightly chain-upgrade suite submitted its gov proposal once the aggregate
RPC reported height >= 1. With 3 of 4 validators forming quorum, the aggregate
can be several blocks ahead of a validator that joined consensus late, whose
local seid then returns NotFound for its own genesis account.

- test: run AwaitNodesAtHeight(1) on every validator before proposing.
- sidecar: signAndBroadcast waits up to 60s while the account lookup
  returns gRPC NotFound before giving up (still non-Terminal).

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@cursor

cursor Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes the sign-and-broadcast account lookup path used by gov tasks; behavior is additive (wait on NotFound) but affects when txs are broadcast on slow-start validators.

Overview
Fixes a startup race where gov sign tasks ran while a late-joining validator’s local seid still had no queryable genesis account (aggregate RPC could be blocks ahead of validator-0).

Sidecar sign-tx: Before signing, retrieveAccount replaces a single AccountNumberSequence call. It retries for up to 60s when the node returns gRPC NotFound, then proceeds or returns the same non-terminal error so task retries still apply. Broadcast is blocked until account number/sequence resolve.

Nightly upgrade integration test: The pre-proposal gate no longer waits on aggregate height ≥ 1 via pollHeightAtLeast; it uses awaitAllValidatorsAtHeight(..., "genesis-await", 1) so each validator’s sidecar confirms local height. awaitAllValidatorsAtHeight gains a step prefix for task names so genesis and post-upgrade gates can both run without collisions.

Tests / deps: Unit tests cover wait-until-found and timeout paths; google.golang.org/grpc is promoted to a direct sidecar dependency for codes/status.

Reviewed by Cursor Bugbot for commit 66a2564. Bugbot is set up for automated code reviews on this repo. Configure here.

@seidroid seidroid Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The nightly upgrade test now waits for each validator to reach height 1 on its own node before proposing, and the sidecar signer now re-polls the account lookup for up to 60s while seid returns gRPC NotFound. I checked against sei-cosmos at the pinned commit that queryABCI returns NotFound unwrapped as status.Error(codes.NotFound, …), so the retry does fire in production, and I found nothing blocking; codex's reading found nothing, which matches mine but added no findings.

1 nit, not posted on the code
  • sidecar/tasks/sign_and_broadcast.go:174 — The NotFound wait applies to every sign-tx kind, not only genesis accounts at startup. A signer key that was never funded now takes about 60s to fail on each task-level attempt instead of failing at once. That is acceptable, but the doc comment could say so, so an operator seeing a slow failure knows to check funding.

seidroid review · decision approve · session 45332683610b4636b4330b728756eeb7 · turn resp_claude_376a4daaeead8ae9526a577f08ca815c · item 7842e2d6d0c4595f838391adc2e98487

Findings: 0 blocking | 0 non-blocking | 0 posted inline

…ners

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

Re the seidroid nit on sign_and_broadcast.go:174: I agree. The doc comment on retrieveAccount now says the wait covers every signer, so a key that was never funded only fails after the full timeout. A slow NotFound failure therefore points to checking the key's funding. Fixed in 66a2564.

@bdchatham
bdchatham merged commit e8bcc38 into main Oct 1, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant