we don't need a walletAddress field but we do need to include the raw message (which contains the address, among other things).
Details
Details
successfully compiled the protos
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Questions inline
| native/cpp/CommonCpp/grpc/protos/identity.proto | ||
|---|---|---|
| 61 ↗ | (On Diff #13548) | message feels too generic |
| 62 ↗ | (On Diff #13548) | Is this a MAC for the message, or something else? Would be good to be more specific here too, I think |
| 66 ↗ | (On Diff #13548) | I thought we had a more specific term we were using here? |
Comment Actions
address feedback
| native/cpp/CommonCpp/grpc/protos/identity.proto | ||
|---|---|---|
| 62 ↗ | (On Diff #13548) | it's the SIWE message that's been signed |
| 66 ↗ | (On Diff #13548) | yeah we switched to AccessToken to refer to the struct in the Identity service, which looks like: pub struct AccessToken {
pub user_id: String,
pub device_id: String,
pub token: String,
pub created: DateTime<Utc>,
pub auth_type: AuthType,
pub valid: bool,
}Since this token maps to the token field in this struct, I think they should have the same name. Maybe we can rename the struct to AccessTokenData and the field to access_token? |
Comment Actions
Maybe we can rename the struct to AccessTokenData and the field to access_token?
That sounds good to me, passing back to you for those changes
Comment Actions
(Might be good for other reviewers to take a look at this... if they're reviewing the rest of the stack, probably good for them to be on this one too)