In D12668#361372, @kamil wrote:

I really don't like the idea (looking at this diff and the entire stack) of passing isKeyserverSession - notification crypto module should treat all devices the same regardless of whether this is a keyserver or client device, and we should only have senderID, because the current design makes it confusing. But given this is implemented and tested I believe we can proceed, you could create a follow-up task to improve it in future but for now LGTM.

Curious for @marcin's perspective on this, and how much time it might take to fix up

In D12668#361448, @ashoat wrote:

In D12668#361372, @kamil wrote:

I really don't like the idea (looking at this diff and the entire stack) of passing isKeyserverSession - notification crypto module should treat all devices the same regardless of whether this is a keyserver or client device, and we should only have senderID, because the current design makes it confusing. But given this is implemented and tested I believe we can proceed, you could create a follow-up task to improve it in future but for now LGTM.

Curious for @marcin's perspective on this, and how much time it might take to fix up

It would require us to implement migration for currently existing users from KEYSERVER prefixed key schema to DEVICE prefixed key schema. I initially thought that it will be easier to skip the migration and treat session separately.

Now I think that migration might not be that hard to implement. However I am worried that treating peer-based and keyserver-based notifications the same way will force us to use the same decryption function for both. Decryption function for peer-based notifications (D12750) is complex and might have bugs. Therefore the advantage of treating them separately is that we guarantee we won't break already existing and working functionality (keyserver-based notifications).

In D12668#364083, @marcin wrote:

In D12668#361448, @ashoat wrote:

In D12668#361372, @kamil wrote:

I really don't like the idea (looking at this diff and the entire stack) of passing isKeyserverSession - notification crypto module should treat all devices the same regardless of whether this is a keyserver or client device, and we should only have senderID, because the current design makes it confusing. But given this is implemented and tested I believe we can proceed, you could create a follow-up task to improve it in future but for now LGTM.

Curious for @marcin's perspective on this, and how much time it might take to fix up

It would require us to implement migration for currently existing users from KEYSERVER prefixed key schema to DEVICE prefixed key schema. I initially thought that it will be easier to skip the migration and treat session separately.

Now I think that migration might not be that hard to implement. However I am worried that treating peer-based and keyserver-based notifications the same way will force us to use the same decryption function for both. Decryption function for peer-based notifications (D12750) is complex and might have bugs. Therefore the advantage of treating them separately is that we guarantee we won't break already existing and working functionality (keyserver-based notifications).

If we skip the work of unifying this logic now, we should at least create a follow-up task to address it later.

In D12668#366328, @ashoat wrote:

In D12668#364083, @marcin wrote:

In D12668#361448, @ashoat wrote:

In D12668#361372, @kamil wrote:

I really don't like the idea (looking at this diff and the entire stack) of passing isKeyserverSession - notification crypto module should treat all devices the same regardless of whether this is a keyserver or client device, and we should only have senderID, because the current design makes it confusing. But given this is implemented and tested I believe we can proceed, you could create a follow-up task to improve it in future but for now LGTM.

Curious for @marcin's perspective on this, and how much time it might take to fix up

It would require us to implement migration for currently existing users from KEYSERVER prefixed key schema to DEVICE prefixed key schema. I initially thought that it will be easier to skip the migration and treat session separately.

Now I think that migration might not be that hard to implement. However I am worried that treating peer-based and keyserver-based notifications the same way will force us to use the same decryption function for both. Decryption function for peer-based notifications (D12750) is complex and might have bugs. Therefore the advantage of treating them separately is that we guarantee we won't break already existing and working functionality (keyserver-based notifications).

If we skip the work of unifying this logic now, we should at least create a follow-up task to address it later.

Created task to track.