-
Notifications
You must be signed in to change notification settings - Fork 19
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
Provide timestamps in identity action updates #1073
Conversation
Your org has enabled the Graphite merge queue for merging into mainAdd the label “queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
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.
This looks good to me
In case you're wondering why we have both a client_timestamp_ns
and a server_timestamp_ns
, maybe this is a good time to explain. When you create the IdentityUpdate
we include a timestamp included the text that gets signed, so that you can do the same action multiple times and get a unique signature. That lets you associate a wallet, revoke it, and then re-associate the same wallet. If two actions have identical signatures, we will reject the second one.
The server_timestamp_ns only comes once you've uploaded the signed request to our nodes.
6c90054
to
a7350fa
Compare
bf76b0c
to
d98b9ac
Compare
ebe5645
to
021f463
Compare
021f463
to
0f3d1b9
Compare
members.insert(identifier, new_member); | ||
members | ||
}, | ||
members: HashMap::from_iter([(identifier, new_member)]), |
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.
Nice cleanup
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.
Heads up @nplasterer this is going to change the group.members()
function to be async, which will need to get updated in iOS and Android.
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.
great work here!
#1044 wont be finished until we expose the timestamps for installations in the bindings inbox_state function, but I advise we do that in a follow up PR since changes in here are quite large this one seem ready to merge 🚀 |
Addresses #1044