feat: Added new setOwner method to establish connection before initialization of the owner - #509
feat: Added new setOwner method to establish connection before initialization of the owner#509roman-nazaruk wants to merge 3 commits into
Conversation
…lization of the owner
peterpeterparker
left a comment
peterpeterparker
left a comment
There was a problem hiding this comment.
Thanks. Few comments. Some new, some similar to those are provided in previous PR #504.
Note that I did not reviewed the tests yet. Let's first focus on the logical part if this makes sense.
| }); | ||
|
|
||
| export type SignerOptions = z.infer<typeof SignerOptionsSchema>; | ||
| export type SignerInitOptions = Omit<SignerOptions, 'owner'>; |
There was a problem hiding this comment.
Created a new separate PR.
| }, 'The value provided is not a valid Identity.'); | ||
|
|
||
| const IdentityNotAnonymousSchema = IdentitySchema.refine( | ||
| export const IdentityNotAnonymousSchema = IdentitySchema.refine( |
There was a problem hiding this comment.
If you export schema that wasn't exposed before, please provides the modification in a separate PR with dedicated tests.
There was a problem hiding this comment.
Created a new separate PR.
| initialized: boolean; | ||
| } { | ||
| try { | ||
| // Use Zod to assert that the owner is not nullish and not anonymous. |
There was a problem hiding this comment.
- I think we can spare this comment.
- No strong opinion but for code consistency, should we use
safeParsehere?
| const {success: isCallCanisterRequest, data: callData} = | ||
| IcrcCallCanisterRequestSchema.safeParse(data); | ||
|
|
||
| if (isNullish(this.#owner)) { |
| */ | ||
| setOwner = ({owner}: {owner: IdentityNotAnonymous}): void => { | ||
| if (nonNullish(this.#owner)) { | ||
| return; |
There was a problem hiding this comment.
IMO we should throw an exception and not silently irgnore it because this might be a misusage.
| }; | ||
|
|
||
| /** | ||
| * Sets owner to signer, thats allows to communicate with a relying party. |
There was a problem hiding this comment.
I would reuse the existing JS Docs that describes the owner in SignerOptionsSchema. To some extent we just move it from the option argument to an internal variable.
/**
* The owner who interacts with the signer.
*
* When the signer is initialized, the owner should be signed in to the consumer dApp.
* Upon signing out, it is up to the consumer to disconnect the signer.
*/
| return; | ||
| } | ||
|
|
||
| const {initialized} = this.handleOwnerInitialized(message); |
2 participants
Motivation
We are updating the oisy-signer so that it now waits for a valid owner to be set before processing non‑read‑only messages, ensuring that it returns a clear NOT_INITIALIZED error when not logged in.
Changes
I removed the need to set the owner during initialization and added a setOwner method to assign it later. Now, if no owner is set when a method that requires it is called, the signer returns a NOT_INITIALIZED error via the notifyError function.
Tests
I updated the tests so that when an owner is not set, the owner-dependent methods return a NOT_INITIALIZED error via notifyError.