Skip to content

Conversation

ilbertt
Copy link
Member

@ilbertt ilbertt commented Jul 23, 2025

Description

Calculates the ingress expiry by first rounding down the timeout and only then applying the clock drift.

As a consequence, adds the deltaInMsAfterThresholdCheck optional parameter to Expiry.fromDeltaInMilliseconds to allow for adding a delta after the internal threshold check.

Blocked by #1075.

How Has This Been Tested?

Same tests should still pass. Added unit tests for addMilliseconds.

Checklist:

  • My changes follow the guidelines in CONTRIBUTING.md.
  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@ilbertt ilbertt requested a review from a team as a code owner July 23, 2025 18:42
Copy link
Contributor

github-actions bot commented Jul 23, 2025

size-limit report 📦

Path Size
@dfinity/agent 47.91 KB (+0.01% 🔺)
@dfinity/assets 52.94 KB (+0.03% 🔺)
@dfinity/auth-client 19.38 KB (0%)
@dfinity/candid 13.22 KB (0%)
@icp-sdk/core 91 B (0%)
@dfinity/identity 18.81 KB (+0.05% 🔺)
@dfinity/identity-secp256k1 33.03 KB (0%)
@dfinity/principal 4.44 KB (0%)
@dfinity/use-auth-client 58.77 KB (-0.02% 🔽)

Base automatically changed from luca/SDK-2234-expiry-calculation to main July 24, 2025 14:31
@ilbertt ilbertt requested review from mraszyk and nathanosdev July 24, 2025 15:05
Copy link
Contributor

@mraszyk mraszyk left a comment

Choose a reason for hiding this comment

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

LGTM semantically, left a few minor comments about naming and documentation

@ilbertt ilbertt requested a review from mraszyk July 25, 2025 09:16
@ilbertt ilbertt merged commit 2a40c57 into main Jul 25, 2025
28 checks passed
@ilbertt ilbertt deleted the luca/SDK-2234-rounding branch July 25, 2025 09:20
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.

3 participants