[uniffi] Add support for custom GroupStateStorage interface

[tomleavy]

Issues:

#81

Description of changes:

• Add a new FFI safe version of GroupStateStorage that will erase generics from the interface.
• Create a Config struct that can take an Arc<dyn GroupStateStorage> as input that works with uniffi and converts it into a GroupStateStorageWrapper that we can then pass to mls-rs upon configuration.

Call-outs:

I am unsure if this actually works in practice yet. The code does generate for Swift, but it looks rather odd (which might just be a quirk of how uniffi handles traits more generally.

Testing:

Keeping this as a draft until someone can create a swift / kotlin / python program that proves the basic strategy is ok.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT license.

[tomleavy]

@mulmarta I think this helps us not go too crazy in the short term. If we can get this to work then we can decide later if mls-rs should erase these generics or not and the change won't break this interface since they are already erased.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 82 lines in your changes are missing coverage. Please review.

Project coverage is 88.69%. Comparing base (d34b971) to head (1b3bb15).

Files	Patch %	Lines
mls-rs-uniffi/src/config/group_state.rs	0.00%	65 Missing ⚠️
mls-rs-uniffi/src/lib.rs	0.00%	16 Missing ⚠️
mls-rs-uniffi/src/config.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
- Coverage   88.89%   88.69%   -0.21%     
==========================================
  Files         172      174       +2     
  Lines       31132    31204      +72     
==========================================
  Hits        27676    27676              
- Misses       3456     3528      +72     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mulmarta]

This will only build in async once

• UniFFI updates -- support of async trait was added a week ago
• async-trait is added to async dependencies of mls-rs-uniffi

[mgeisler]

I tried using the code here from Python via #87. I got the error @mulmarta talked about:

Traceback (most recent call last):
  File "/tmp/run-python-rj1JKa/script.py", line 17, in <module>
    client = Client(b"alice", signature_keypair, client_config)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/run-python-rj1JKa/mls-rs-uniffi-5cb1d76f88c59931/mls_rs_uniffi.py", line 1236, in __init__
    _UniffiConverterTypeClientConfig.lower(client_config))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/run-python-rj1JKa/mls-rs-uniffi-5cb1d76f88c59931/mls_rs_uniffi.py", line 428, in lower
    cls.write(value, builder)
  File "/tmp/run-python-rj1JKa/mls-rs-uniffi-5cb1d76f88c59931/mls_rs_uniffi.py", line 2475, in write
    _UniffiConverterTypeGroupStateStorage.write(value.group_state_storage, buf)
  File "/tmp/run-python-rj1JKa/mls-rs-uniffi-5cb1d76f88c59931/mls_rs_uniffi.py", line 2091, in write
    buf.write_u64(cls.lower(value))
                  ^^^^^^^^^^^^^^^^
  File "/tmp/run-python-rj1JKa/mls-rs-uniffi-5cb1d76f88c59931/mls_rs_uniffi.py", line 2080, in lower
    return value._uniffi_clone_pointer()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/run-python-rj1JKa/mls-rs-uniffi-5cb1d76f88c59931/mls_rs_uniffi.py", line 1990, in _uniffi_clone_pointer
    return _rust_call(_UniffiLib.uniffi_mls_rs_uniffi_fn_clone_groupstatestorage, self._pointer)
                                                                                  ^^^^^^^^^^^^^
AttributeError: 'PythonGroupStateStorage' object has no attribute '_pointer'
test sync_tests::test_client_new ... FAILED

I looked some more and the support for foreign traits (implementing a Rust trait in another language) seems very new:

• FFI callbacks and vtables mozilla/uniffi-rs#1818
• Make foreign trait implementations opt-in (#1827) mozilla/uniffi-rs#1902
• Consolidate/Simplify the foreign bindings handlemap code mozilla/uniffi-rs#1969
• Adding FfiType::Handle and use it for Rust futures mozilla/uniffi-rs#1972

(and possibly more).

The second of these made me aware of a #[uniffi::export(with_foreign)] syntax: the with_foreign attribute corresponds to this short section in the unreleased manual.

It's not clear to me what the overall state of all this is, but if I use #[uniffi::export(with_foreign)] on pub trait GroupStateStorage, I'm forced to change the &[u8] types to Vec<u8> (seems like a good change, since it indicates that the type takes ownership, which you would expect when passed in from Python).

When generate the Python bindings, GroupStateStorage now looks like this:

class GroupStateStorage(typing.Protocol):

    def state(self, group_id: "bytes"):
        raise NotImplementedError

    def epoch(self, group_id: "bytes", epoch_id: "int"):
        raise NotImplementedError

    def write(self, state: "GroupState",
              epoch_inserts: "typing.List[EpochRecord]",
              epoch_updates: "typing.List[EpochRecord]"):
        raise NotImplementedError

    def max_epoch_id(self, group_id: "bytes"):
        raise NotImplementedError

before it looked like this:

class GroupStateStorage:

    _pointer: ctypes.c_void_p

    def __del__(self):
        # In case of partial initialization of instances.
        pointer = getattr(self, "_pointer", None)
        if pointer is not None:
            _rust_call(_UniffiLib.uniffi_mls_rs_uniffi_fn_free_groupstatestorage, pointer)

    def _uniffi_clone_pointer(self):
        return _rust_call(_UniffiLib.uniffi_mls_rs_uniffi_fn_clone_groupstatestorage, self._pointer)

I can now generate a Client using an empty Python implementation of the storage trait:

from mls_rs_uniffi import *

signature_keypair = generate_signature_keypair(CipherSuite.CURVE25519_AES128)

class PythonGroupStateStorage(GroupStateStorage):
    def state(self, group_id: "bytes"):
        raise NotImplementedError
    def epoch(self, group_id: "bytes",epoch_id: "int"):
        raise NotImplementedError
    def write(self, state: "GroupState",epoch_inserts: "typing.List[EpochRecord]",epoch_updates: "typing.List[EpochRecord]"):
        raise NotImplementedError
    def max_epoch_id(self, group_id: "bytes"):
        raise NotImplementedError

group_state_storage = PythonGroupStateStorage()
client_config = ClientConfig(group_state_storage)
client = Client(b"alice", signature_keypair, client_config)

I pushed the change here: mgeisler@35b19d5.

[tomleavy]

Hey @mgeisler funny I was deep into looking through issues on uniffi and found the with_foreign stuff as well. Made the edits and now I think we are good!

I also added async_trait in there, so if you patch in mainline uniffi it should work (I think)

[mulmarta]

Hey @mgeisler funny I was deep into looking through issues on uniffi and found the with_foreign stuff as well. Made the edits and now I think we are good!

I also added async_trait in there, so if you patch in mainline uniffi it should work (I think)

The need to patch in mainline uniffi also breaks the CI. You can ignore the test for now https://github.com/awslabs/mls-rs/blob/main/mls-rs-uniffi/src/lib.rs#L594

[tomleavy]

@mulmarta removed the file / ignored the test for now to avoid rewriting it. Lets just wait until uniffi updates and rewrite the test based on whatever progress we make on the API in the meantime