Skip to content

Fix fetch certs interval#3430

Merged
sea-snake merged 7 commits intomainfrom
sea-snake/fix-fetch-certs-interval
Oct 28, 2025
Merged

Fix fetch certs interval#3430
sea-snake merged 7 commits intomainfrom
sea-snake/fix-fetch-certs-interval

Conversation

@sea-snake
Copy link
Copy Markdown
Contributor

@sea-snake sea-snake commented Oct 21, 2025

As observed by @peterpeterparker, earlier retry delays only applied to the first outcall error instead of any failed future outcalls.

Thanks @peterpeterparker for catching this one.

Changes

  • Switch around meaning of delay argument in schedule_fetch_certs, None now default to FETCH_CERTS_INTERVAL instead of 0.
  • When outcall succeeds, schedule_fetch_certs is called with None:
    • Will default to FETCH_CERTS_INTERVAL with timer.
    • Will use 60 as retry delay.
  • When outcall fails, schedule_fetch_certs is called with Some:
    • Will use delay with timer.
    • Will use min(FETCH_CERTS_INTERVAL, max(60, delay * 2)) as retry delay.

Tests

Added should_compute_next_certs_fetch_delay test to make sure delay is computed correctly.

…first http outcall error instead of any failed future outcalls.
…first http outcall error instead of any failed future outcalls.
# Conflicts:
#	src/internet_identity/src/openid/google.rs
@sea-snake sea-snake marked this pull request as ready for review October 28, 2025 10:48
@sea-snake sea-snake requested a review from lmuntaner October 28, 2025 10:48
Copy link
Copy Markdown
Contributor

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

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

Nitpick but already approving.

Comment thread src/internet_identity/src/openid/generic.rs Outdated
Comment thread src/internet_identity/src/openid/generic.rs Outdated
@sea-snake sea-snake requested a review from lmuntaner October 28, 2025 11:19
Copy link
Copy Markdown
Contributor

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

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

Thanks!

@sea-snake
Copy link
Copy Markdown
Contributor Author

@lmuntaner Also renamed to MAX_VALIDITY_WINDOW to MAX_VALIDITY_WINDOW_SECONDS. Keep in mind that it was previously nanoseconds, had to update it's usage in two places with secs_to_nanos().

@sea-snake sea-snake requested a review from lmuntaner October 28, 2025 11:20
@sea-snake sea-snake enabled auto-merge October 28, 2025 11:24
@sea-snake sea-snake added this pull request to the merge queue Oct 28, 2025
Merged via the queue into main with commit bb0c790 Oct 28, 2025
75 of 76 checks passed
@sea-snake sea-snake deleted the sea-snake/fix-fetch-certs-interval branch October 28, 2025 11:37
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.

2 participants