Repository navigation
Conversation
… precision checkRIExchangeCloudCheck and validateLadderUnverifiedFail refuse a cloud check that does not postdate the row's last change, but compared the operator's nanosecond checked_at against an updated_at that Postgres had truncated to microseconds. A check stamped at the change's own instant read as a few hundred nanoseconds after it and was accepted, which is what TestPostgresStoreDB_ResolveStaleRIExchange_ValidatesCloudCheck catches now that LeanerCloud#1165 runs it against the shared container. Truncate checked_at to microseconds before the "after the last change" comparison in both paths (checkPostdatesDBTime), and add unit tests that pin the boundary without a database. Closes LeanerCloud#1251 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
This was referenced Oct 23, 2026
Draft
This was referenced Oct 23, 2026
Draft
Draft
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the red Integration Tests job on main (
a14dc8f2, run 39878015423), caused by our #1109.Closes #1251
Root cause
checkRIExchangeCloudCheckrefuses a cloud check that doesn't postdate the exchange's last change, using!c.CheckedAt.After(row.updatedAt). Butrow.updatedAtcomes back from Postgres truncated to microseconds, whileCheckedAtis a Go time with nanoseconds. So a check stamped at the change's own instant reads as a few hundred nanoseconds after it and is accepted.TestPostgresStoreDB_ResolveStaleRIExchange_ValidatesCloudCheck/checked_at_the_changecovers exactly this case. The bad resolve also releases the shared hold, so the remaining subtests fail after it. That's why CI shows all 8 subtests failing.The test only started running on main recently. On #1109's branch it skipped unless
RI_EXCHANGE_TEST_DATABASE_URLwas set, and #1165 then moved it onto the shared testcontainers Postgres. Our pre-merge verification for #1109 rango test -shortwithout the integration tag, so it didn't catch this either.Fix
checkPostdatesDBTime(checked, last)ininternal/config/store_postgres_ri_exchange_resolve.go. It truncatescheckedto microseconds before the strict "after" comparison, so a check taken within the same microsecond as the change can't be ordered after it.checkRIExchangeCloudCheckand invalidateLadderUnverifiedFail(store_postgres_ladder_resolve.go). The ladder path has the same pattern, because itslastChangeis the newest chain node'supdated_atfrom Postgres.Real-world impact is small. The handlers parse
checked_atas RFC 3339, normally whole seconds, so only sub-microsecond inputs were affected. The fix makes the boundary match what the comment and the test already promise.Verification
Local, Linux, Docker Postgres via testcontainers,
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null:TestCheckPostdatesDBTime,TestCheckRIExchangeCloudCheck_CheckedAtTheChangeIsRefusedandTestValidateLadderUnverifiedFail_CheckedAtTheChangeIsRefusedall fail with main's comparisons swapped back in, and pass with the fix.go test -tags integration -count=1 -run 'ResolveStaleRIExchange|LadderResolve|ResolveLadder' ./internal/config/passed on 3 consecutive runs, includingValidatesCloudCheckand the concurrency tests (ConcurrentResolvesOneWins,RaceWithLateSettlementKeepsHold). On origin/main the sameValidatesCloudCheckrun fails with the CI error above.go build ./...;go vetwith and without-tags integrationon./internal/config/;go test ./internal/config/ ./internal/mocks/.--build-tags integration.gocyclo -over 10is silent on both changed files.scripts/setup-git-secrets.shpatterns, run in a fresh repo with an isolated HOME, are clean on the 4 changed files, and a planted key was flagged.scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.I didn't rerun the full
internal/configintegration package. The two resolve suites above are the ones that touch the changed code.Out of scope
Main's other red checks: the GO-2026-6809 govulncheck steps (#1236), and a transient
sum.golang.orgTLS failure while installing actionlint in pre-commit.Labels: #1251 has
type/bug; please add priority/severity as you see fit.🤖 Generated with Claude Code