Skip to content
This repository was archived by the owner on Dec 3, 2025. It is now read-only.

Chore/watch subs default#56

Merged
devceline merged 15 commits intomainfrom
chore/watch-subs-default
Sep 29, 2023
Merged

Chore/watch subs default#56
devceline merged 15 commits intomainfrom
chore/watch-subs-default

Conversation

@devceline
Copy link
Copy Markdown
Contributor

@devceline devceline commented Sep 27, 2023 •
edited
Loading

Resolves #55

  • Adds lastWatchedAccount store that should only ever store no values or 1 value (which would be the last watched account)
  • Once an account is watched, it is registered as the lastWatchedAccount
  • On every init, the last watched account is checked. If one exists and it has a valid identity key, attempt to watch for it.
  • Account for this change in the did.json cache unit test since there will be 2 new calls 1 occurring from watchSubscriptions and the other occurring from handling the response.

@devceline devceline marked this pull request as ready for review September 27, 2023 11:50
@devceline devceline requested review from Cali93 and bkrem September 27, 2023 11:50
bkrem
bkrem reviewed Sep 27, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@bkrem bkrem left a comment

Choose a reason for hiding this comment

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

Nice one, LGTM generally 👍

Some minor notes and think we should publish a canary to dogfood it downstream in the hooks to see if we see any edge cases or scenarios currently not accounted for.

Comment thread packages/notify-client/src/controllers/engine.ts Outdated
Comment thread packages/notify-client/src/controllers/engine.ts Outdated
Comment thread packages/notify-client/src/controllers/engine.ts Outdated
Comment thread packages/notify-client/src/controllers/engine.ts Outdated
Comment thread packages/notify-client/test/clients.spec.ts Outdated
Comment thread packages/notify-client/test/clients.spec.ts
devceline and others added 5 commits September 27, 2023 14:54
@devceline @bkrem
Update packages/notify-client/src/controllers/engine.ts
ec5c289 
Co-authored-by: Ben Kremer <ben@walletconnect.com>
@devceline @bkrem
Update packages/notify-client/src/controllers/engine.ts
09cc9cb 
Co-authored-by: Ben Kremer <ben@walletconnect.com>
@devceline @bkrem
Update packages/notify-client/test/clients.spec.ts
d78b298 
Co-authored-by: Ben Kremer <ben@walletconnect.com>
@devceline @bkrem
Update packages/notify-client/src/controllers/engine.ts
078c873 
Co-authored-by: Ben Kremer <ben@walletconnect.com>
@devceline
chore: wrap identity throw in if braces
de77bca
Cali93
Cali93 approved these changes Sep 27, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Cali93 Cali93 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread packages/notify-client/src/controllers/engine.ts
Comment thread packages/notify-client/src/controllers/engine.ts Outdated
@devceline devceline merged commit 7dd05e5 into main Sep 29, 2023
@devceline devceline deleted the chore/watch-subs-default branch September 29, 2023 10:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@bkrem bkrem bkrem left review comments

@Cali93 Cali93 Cali93 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Watch Subscriptions on Init

3 participants

@devceline @bkrem @Cali93