gets the keys for all devices belonging to a given user
Details
Details
registered a new user and then called this RPC. got back the keys I sent in the register request, including one-time content and notif keys
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- master
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Looks good to me! I would love it if someone more experienced in Rust would take a look, @bartek can you take a final round?
| services/identity/src/database.rs | ||
|---|---|---|
| 412–421 ↗ | (On Diff #31654) | Just wondering, if we call get_one_time_key, the task will be spawned, and get_one_time_key will be called second time before new keys are uploaded, will this cause any harm? |
| services/identity/src/client_service.rs | ||
|---|---|---|
| 793–803 ↗ | (On Diff #31654) | Nit, but might increase readability by having local use use outbound_keys_for_user_request::Identifier; let (user_ident, auth_type) = match message.identifier { None => { return Err(tonic::Status::invalid_argument("no identifier provided")) } Some(Identifier::Username(username)) => (username, AuthType::Password), Some(Identifier::WalletAddress(address)) => (address, AuthType::Wallet), }; |
| services/identity/src/database.rs | ||
| 412–421 ↗ | (On Diff #31654) | Valid point. Two tasks will be spawned and thus two RefreshKeyRequest messages will be sent through Tunnelbroker to the device. |
Comment Actions
Requesting changes just to confirm our doubts (one question inline), besides that: LGTM
| services/identity/src/database.rs | ||
|---|---|---|
| 412–421 ↗ | (On Diff #31654) | In our Olm's fork @kamil found this: https://github.com/CommE2E/olm/blob/6d0eaa993257225faf62768c898fe090fcc2dd2d/include/olm/olm.h#L243
So it looks like this is safe and we're just going to publish more one-time keys, without invalidating existing ones, am I right? |
| services/identity/src/database.rs | ||
|---|---|---|
| 412–421 ↗ | (On Diff #31654) | Olm Account keeps one time keys in a List collection implemented here: https://github.com/CommE2E/olm/blob/6d0eaa993257225faf62768c898fe090fcc2dd2d/include/olm/list.hh. This list is parameterized with its maximal capacity. When new element is inserted into this list and the number of elements stored is already equal to maximal capacity then the oldest element (in FIFO manner) is discarded. Olm Account uses MAX_ONE_TIME_KEYS constant as maximal capacity: https://github.com/CommE2E/olm/blob/main/include/olm/account.hh#L50, which appears to be equal to 100 https://github.com/CommE2E/olm/blob/6d0eaa993257225faf62768c898fe090fcc2dd2d/include/olm/account.hh#L45. Generating new one time keys simple inserts new elements to the list https://github.com/CommE2E/olm/blob/6d0eaa993257225faf62768c898fe090fcc2dd2d/src/account.cpp#L292. That said successive calls to generate_one_time_keys will discard oldest one time keys if there are already 100 one time keys. |