feat: implement transaction [WPB-11400] [WPB-10917] #732

SimonThormeyer · 2024-10-23T15:08:51Z

What's new in this PR

PR Submission Checklist for internal contributors

The PR Title
- conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
- contains a reference JIRA issue number like SQPIT-764
- answers the question: If merged, this PR will: ... ³

https://sparkbox.com/foundry/semantic_commit_messages
https://github.com/wireapp/.github#usage
E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

codecov-commenter · 2024-10-24T10:16:49Z

Codecov Report

Attention: Patch coverage is 58.60450% with 1234 lines in your changes missing coverage. Please review.

Project coverage is 72.15%. Comparing base (b5f0e7a) to head (82b335e).

Files with missing lines	Patch %	Lines
crypto-ffi/src/generic/context/mod.rs	0.00%	360 Missing ⚠️
crypto-ffi/src/generic/mod.rs	0.00%	320 Missing ⚠️
crypto-ffi/src/generic/context/e2ei.rs	0.00%	183 Missing ⚠️
crypto/src/mls/conversation/commit.rs	82.73%	67 Missing ⚠️
crypto-ffi/src/generic/context/proteus.rs	0.00%	64 Missing ⚠️
crypto/src/e2e_identity/init_certificates.rs	61.76%	52 Missing ⚠️
crypto/src/e2e_identity/conversation_state.rs	81.14%	33 Missing ⚠️
crypto/src/e2e_identity/rotate.rs	89.61%	30 Missing ⚠️
crypto/src/mls/conversation/decrypt.rs	93.00%	30 Missing ⚠️
crypto/src/mls/conversation/db_count.rs	7.14%	26 Missing ⚠️
... and 9 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #732      +/-   ##
==========================================
- Coverage   76.19%   72.15%   -4.05%     
==========================================
  Files          99      104       +5     
  Lines       19608    19689      +81     
==========================================
- Hits        14941    14207     -734     
- Misses       4667     5482     +815

Files with missing lines	Coverage Δ
crypto-attributes/src/lib.rs	`87.17% <ø> (ø)`
crypto-ffi/src/lib.rs	`0.00% <ø> (ø)`
crypto/src/e2e_identity/refresh_token.rs	`100.00% <100.00%> (ø)`
crypto/src/e2e_identity/stash.rs	`93.47% <100.00%> (+0.07%)`	⬆️
crypto/src/error.rs	`23.80% <ø> (ø)`
crypto/src/lib.rs	`37.50% <ø> (-37.50%)`	⬇️
crypto/src/mls/buffer_external_commit.rs	`87.67% <100.00%> (+1.44%)`	⬆️
crypto/src/mls/client/id.rs	`59.25% <ø> (ø)`
crypto/src/mls/conversation/buffer_messages.rs	`90.21% <100.00%> (+1.65%)`	⬆️
crypto/src/mls/conversation/commit_delay.rs	`91.66% <100.00%> (+0.92%)`	⬆️
... and 75 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5f0e7a...82b335e. Read the comment docs.

And mark corresponding methods on CoreCrypto as deprecated

The SQLite in-memory instance is initiated with an empty path string, which had lead to empty query results in the iOS-specific keychain interactions.

This includes removal of the mls-keystore cargo feature

This will allow us to drop the deprecated API part in wasm bindings

...when catching an error during a transaction

To make the error thrown by the wrapper consistent with the ones before the transaction feature.

istankovic · 2024-11-07T11:08:45Z

crypto-ffi/bindings/jvm/src/main/kotlin/com/wire/crypto/client/CoreCryptoCentral.kt

        }
    })
+    if (error != null) {
+        throw error as Throwable
+    }


Do we have a test for this?

istankovic · 2024-11-07T11:08:55Z

crypto-ffi/bindings/js/CoreCrypto.ts

+        });
+        if (error !== null) {
+            throw error;
+        }


Do we have a test for this?

istankovic · 2024-11-07T11:14:38Z

crypto-ffi/bindings/js/test/CoreCrypto.test.js

+        return error;
+    });
+
+    expect(error.rustStackTrace).toBe("CryptoError(InvalidContext)");


Thank you for this test! 💜

echoes-hq bot added the echoes: features End-user visible changes intended to create customer value label Oct 23, 2024

SimonThormeyer force-pushed the feat/transaction branch 2 times, most recently from 1f1ae57 to 77ca951 Compare October 24, 2024 06:51

SimonThormeyer force-pushed the feat/transaction branch 26 times, most recently from 01f4f8c to b9afcf6 Compare October 25, 2024 12:49

SimonThormeyer added 26 commits November 7, 2024 11:02

chore: expose context api for proteus functions in wasm wrapper

0a9a713

And mark corresponding methods on CoreCrypto as deprecated

chore: fix interop test

c888ba7

chore: update interop ffi client to 1.0.2 api

73d6f42

chore: use ffi client in interop test

43e37e3

style: run cargo fmt

89c1502

chore: fix compile errors in e2ei_rotate()

5cd9814

chore: add e2ei_rotate() to context and bindings

b3cde2a

chore: remove transaction lock

f2d2c16

chore: fix wasm close and wipe

3b5f6fb

chore: fix corrupted transaction state after commit_accepted()

2c4a2b8

chore: fix transaction creation for iOS

a030fef

The SQLite in-memory instance is initiated with an empty path string, which had lead to empty query results in the iOS-specific keychain interactions.

chore: decouple dynamic dispatch boiler plate in transaction

9a7e060

chore: dynamic dispatch module in compliance with cargo features

069a586

chore: compliance with proteus feature in context

231129b

chore: fix independent compilation of cargo features

cfde9e6

This includes removal of the mls-keystore cargo feature

chore: fix transactions in bindings

44ae62a

chore: incorporate fix in uniffi bindings

9068533

chore: use transaction api in deprecated functions in TS wrapper

4174af6

This will allow us to drop the deprecated API part in wasm bindings

chore: remove deprecated functions from wasm bindings

e69254c

chore: keep proteus error code from context

5b5d3cd

...when catching an error during a transaction

chore: handle transaction error in typescript wrapper

618ee4e

To make the error thrown by the wrapper consistent with the ones before the transaction feature.

chore: handle transaction error in kotlin wrapper

96ef935

chore: fix benches

2b2b306

chore: remove transactional crypto provider

e5e8a12

chore: fix docs

7ce86a2

chore: test that interaction with invalid context throws error

82b335e

SimonThormeyer force-pushed the feat/transaction branch from 00128e2 to 82b335e Compare November 7, 2024 10:05

istankovic reviewed Nov 7, 2024

View reviewed changes

crypto-ffi/bindings/js/CoreCrypto.ts

});

if (error !== null) {

throw error;

}

Copy link

Member

istankovic Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a test for this?

istankovic reviewed Nov 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement transaction [WPB-11400] [WPB-10917] #732

feat: implement transaction [WPB-11400] [WPB-10917] #732

SimonThormeyer commented Oct 23, 2024

codecov-commenter commented Oct 24, 2024 •

edited

Loading

istankovic Nov 7, 2024

istankovic Nov 7, 2024

istankovic Nov 7, 2024

feat: implement transaction [WPB-11400] [WPB-10917] #732

Are you sure you want to change the base?

feat: implement transaction [WPB-11400] [WPB-10917] #732

Conversation

SimonThormeyer commented Oct 23, 2024

What's new in this PR

PR Submission Checklist for internal contributors

codecov-commenter commented Oct 24, 2024 • edited Loading

Codecov Report

istankovic Nov 7, 2024

Choose a reason for hiding this comment

istankovic Nov 7, 2024

Choose a reason for hiding this comment

istankovic Nov 7, 2024

Choose a reason for hiding this comment

codecov-commenter commented Oct 24, 2024 •

edited

Loading