Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: fetch root key before making calls #966

Merged
merged 5 commits into from
Feb 1, 2025

Conversation

krpeacock
Copy link
Contributor

@krpeacock krpeacock commented Jan 25, 2025

Description

Since the root key cannot be fetched inside a synchronous constructor, we should await the root key when the shouldFetchRootKey flag is set.

This PR adds a guard to the HttpAgent async calls to make sure the root key call has finished before making the call. It also modifies some checks for rootKey in the canisterStatus and actor utils that should instead wait for the call to be completed.

This shores up occasional inconsistencies with the agent during local development, and that have affected projects in our examples repostory

Fixes # SDK-1905

How Has This Been Tested?

New tests added to the HttpAgent unit test suite

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.

Copy link
Contributor

github-actions bot commented Jan 25, 2025

size-limit report 📦

Path Size
@dfinity/agent 72.38 KB (+0.41% 🔺)
@dfinity/candid 11.65 KB (0%)
@dfinity/principal 4.16 KB (0%)
@dfinity/auth-client 50.33 KB (+0.49% 🔺)
@dfinity/assets 67.92 KB (+0.1% 🔺)
@dfinity/identity 47.94 KB (+0.42% 🔺)
@dfinity/identity-secp256k1 106.42 KB (+0.1% 🔺)

@krpeacock krpeacock marked this pull request as ready for review January 29, 2025 22:01
@krpeacock krpeacock requested a review from a team as a code owner January 29, 2025 22:01
@krpeacock krpeacock requested a review from nathanosdev January 29, 2025 22:03
@krpeacock
Copy link
Contributor Author

There is one arguably breaking change - the rootKey is now typed as ArrayBuffer | null. However, I'd say that this typing makes the reality of the workflow explicit. In the past, we would always set the rootKey to the IC_ROOT_KEY regardless of whether we were going to fetch one or not, leading to the potential asynchronous errors we've already encountered.

This does mean that according to TypeScript, the HttpAgent exported in this version wouldn't be usable in an Actor from version 2.0.0. However, we generally see the reverse pattern in practice. Generally people use an up-to-date Actor with an outdated agent.

I think this can be shipped as a minor version update, for this reason.

@krpeacock krpeacock requested a review from nathanosdev January 30, 2025 18:42
@krpeacock krpeacock merged commit 298c772 into main Feb 1, 2025
17 checks passed
@krpeacock krpeacock deleted the kaia/await-fetch-root-key-calls branch February 1, 2025 00: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