-
Notifications
You must be signed in to change notification settings - Fork 981
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/wallet sync #13308
Feat/wallet sync #13308
Conversation
Jenkins BuildsClick to see older builds (50)
|
c8edd0a
to
d2dfef5
Compare
(assoc existing-accs idx new-acc) | ||
:else existing-accs))) | ||
new-accounts (reduce reduce-fn (vec existing-accounts) (rpc->accounts accounts))] | ||
(fx/merge cofx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant fx/merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
(let [address (:address new-acc) | ||
removed? (:removed new-acc) | ||
; Find index of existing account with given address (might be nil) | ||
idx (first (keep-indexed (fn [idx item] (when (= (:address item) address) idx)) existing-accs))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would suggest to just create a map with address keys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in that case you could just test if address exists, and assoc dissoc by address
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed. I was vaguely concerned about ordering (?). Whatever, it's much simpler now.
70aea6e
to
6655c0d
Compare
99% of end-end tests have passed
Not executed tests (1)Failed tests (1)Click to expand
Passed tests (83)Click to expand
|
79% of end-end tests have passed
Not executed tests (1)Failed tests (18)Click to expand
Passed tests (66)Click to expand
|
87% of end-end tests have passed
Not executed tests (1)Failed tests (11)Click to expand
Passed tests (73)Click to expand
|
90% of end-end tests have passed
Not executed tests (1)Failed tests (8)Click to expand
Passed tests (76)Click to expand
|
@siphiuel |
Depends on status-im/status-go#2607.
This works for watch-only accounts only (sync for all types of wallet accounts will be implemented separately).
Also fixes an issue introduced by #13315 (watch-only wallet accounts could not be deleted).
Steps that were taken to verify the PR:
Sync All Devices
button on Device 1). Confirm wallet account appears on Device 2.