update clients' key package refs upon committing

[stefanwire]

https://wearezeta.atlassian.net/browse/FS-878

The logic has been manually tested. Automated tests will be added/integrated by FS-797.

Checklist

• The PR Title explains the impact of the change.
• The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
• changelog.d contains the following bits of information (details):
  • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.
  • If internal end-points have been added or changed: which services have to be deployed in a specific order?

[mdimjasevic]

I think you're pretty much there. I left a few comments inlined.

[mdimjasevic]

Can you also add tests? It would be good to have one for the case when there's no key package in the update path and then one for the case when there is a key package.

[stefanwire]

Can you also add tests? It would be good to have one for the case when there's no key package in the update path and then one for the case when there is a key package.

I'm not sure whether we can do this using the mls-test-cli. There's always an update path in a commit from what I see. 🤔

[mdimjasevic]

I'm not sure whether we can do this using the mls-test-cli. There's always an update path in a commit from what I see. thinking

We're not there yet (or at least I haven't thought this through), but it would be nice if we didn't have to write an integration test for everything we do. With Galley polysemized, I think we ought to start thinking how to make testing easy without needing to reach for another integration test. For this particular scenario, perhaps it would be too much to construct by hand a binary representation of a commit so we can parseMLS it; with Paolo's serialisation PR #2597 , hopefully this will be a breeze.

Not every commit has an update path - it is an optional field.

[stefanwire]

We're not there yet (or at least I haven't thought this through), but it would be nice if we didn't have to write an integration test for everything we do. With Galley polysemized, I think we ought to start thinking how to make testing easy without needing to reach for another integration test. For this particular scenario, perhaps it would be too much to construct by hand a binary representation of a commit so we can parseMLS it; with Paolo's serialisation PR #2597 , hopefully this will be a breeze.

Thanks for the pointer!

Not every commit has an update path - it is an optional field.

True and already acknowledged in code. 👍

[mdimjasevic]

I really think the PR should come with tests. One option is to see if OpenMLS supports explicitly leaving out/including an update path. Another one, which I've been doing in another PR, is to construct the payload directly in Haskell; sure, that's a bit tedious to do by hand until we get Paolo's serialisation PR merged, but it should be doable. And you can always cherry-pick from Paolo's branch.

[mdimjasevic]

Not happy with this, but I'm letting this to be merged without any tests.

On a more general note, testing changes to the application code ought to be easy. We should figure out how to do that easily with Polysemy.