Update to use 0.25.0 inapp sdk #34
Conversation
WalkthroughAdds TEE operator support and locale flags; extends ClientFeatureOverrides with multiple timeout/interceptor/TEE fields; updates pigeon schema and generated bindings; swaps GNARK dependency for a TEE operator; wires new fields through the verifier module; adds XCFramework creation to the iOS build script. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
scripts/setup_vendor.sh (1)
16-19: Consider renaming the environment variable for clarity.The variable
OVERRIDE_GNARK_PROVER_GITis now used to control the git ref forreclaim_tee_operator_flutter, but the name still references "GNARK_PROVER". Consider renaming it toOVERRIDE_TEE_OPERATOR_GITor a more generic name likeOVERRIDE_OPERATOR_GITto better reflect its current purpose.Apply this diff:
-if [[ -z "$OVERRIDE_GNARK_PROVER_GIT" ]]; then - OVERRIDE_GNARK_PROVER_GIT="main" +if [[ -z "$OVERRIDE_TEE_OPERATOR_GIT" ]]; then + OVERRIDE_TEE_OPERATOR_GIT="main" fi if [[ -z "$OVERRIDE_RECLAIM_FLUTTER_SDK_GIT" ]]; then OVERRIDE_RECLAIM_FLUTTER_SDK_GIT="main" fi echo "OVERRIDE_RECLAIM_FLUTTER_SDK_GIT=$OVERRIDE_RECLAIM_FLUTTER_SDK_GIT" -echo "OVERRIDE_GNARK_PROVER_GIT=$OVERRIDE_GNARK_PROVER_GIT" +echo "OVERRIDE_TEE_OPERATOR_GIT=$OVERRIDE_TEE_OPERATOR_GIT" echo "dependency_overrides: reclaim_tee_operator_flutter: git: url: https://$PACKAGE_CLONE_USER:[email protected]/reclaimprotocol/reclaim-tee-operator-flutter.git - ref: $OVERRIDE_GNARK_PROVER_GIT + ref: $OVERRIDE_TEE_OPERATOR_GITpubspec.yaml (1)
32-40: Consider pinning git dependencies to specific commits or tags.The git dependencies are currently pointing to the
mainbranch without version pinning. This can lead to non-deterministic builds and make it difficult to reproduce issues, as different builds may pull different versions of the dependencies.Consider using specific commit SHAs or version tags for better reproducibility:
reclaim_inapp_sdk: git: url: [email protected]:reclaimprotocol/reclaim-inapp-sdk.git ref: <specific-commit-sha-or-tag> reclaim_tee_operator_flutter: git: url: [email protected]:reclaimprotocol/reclaim-tee-operator-flutter.git ref: <specific-commit-sha-or-tag>lib/reclaim_verifier_module.dart (1)
12-14: Avoid using implementation imports from external packages.The code imports from the internal
src/common/download/download.dartpath ofreclaim_tee_operator_flutter. Accessing implementation details breaks encapsulation and makes the code fragile to upstream changes. IfdownloadWithHttpis needed by consumers, it should be exported publicly by thereclaim_tee_operator_flutterpackage.Consider requesting that
reclaim_tee_operator_flutterexportsdownloadWithHttpas part of its public API, or copy the required functionality into this module if the upstream maintainers don't intend to expose it publicly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
generated/android/src/main/java/org/reclaimprotocol/inapp_sdk/Messages.ktis excluded by!**/generated/**generated/ios/Sources/ReclaimInAppSdk/Messages.his excluded by!**/generated/**generated/ios/Sources/ReclaimInAppSdk/Messages.mis excluded by!**/generated/**generated/ios/Sources/ReclaimInAppSdk/Messages.swiftis excluded by!**/generated/**pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)lib/reclaim_verifier_module.dart(4 hunks)lib/src/pigeon/messages.pigeon.dart(39 hunks)pigeon/schema.dart(3 hunks)pubspec.yaml(2 hunks)scripts/setup_vendor.sh(1 hunks)
🔇 Additional comments (7)
CHANGELOG.md (1)
1-8: LGTM!The changelog entry accurately documents the new 0.25.0 features including locale support, TEE+MPC protocol integration, and expanded API surface.
lib/reclaim_verifier_module.dart (2)
454-460: LGTM!The feature flag configuration correctly populates the new timeout and interceptor fields from the
ClientFeatureOverridesmodel.
590-608: LGTM!The verification options are correctly logged and propagated, including the new
localeanduseTeeOperatorfields. The implementation properly handles the new configuration surface.pigeon/schema.dart (2)
80-128: LGTM!The
ClientFeatureOverridesschema correctly expands the configuration surface with new optional timeout and interceptor fields. All fields are appropriately nullable with clear documentation.
187-232: LGTM!The
ReclaimApiVerificationOptionsschema additions are well-documented. TheuseTeeOperatorfield clearly explains the three-state behavior (true/false/null) and thelocalefield provides clear guidance on localization.lib/src/pigeon/messages.pigeon.dart (2)
278-387: LGTM!The autogenerated code correctly implements the expanded
ClientFeatureOverridesdata model with proper serialization and deserialization for all new fields.
561-650: LGTM!The autogenerated code correctly implements the expanded
ReclaimApiVerificationOptionswithlocaleanduseTeeOperatorfields, including proper serialization/deserialization logic.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/reclaim_verifier_module.dart (2)
12-15: Internal import fromreclaim_tee_operator_flutteris a stability trade-offUsing
downloadWithHttpfromsrc/common/download/download.dart(with// ignore: implementation_imports) is a deliberate dependency on internal package structure. This is fine short term, but any upstream refactor of internals can break this module without a version-constraint change; consider exposing this via the public API ofreclaim_tee_operator_flutterlonger term.
583-611: Verification options: propagation of new fields looks correctLogging and
copyWithnow includeclaimCreationType,isCloseButtonVisible,locale, anduseTeeOperator, with the enum converted viaClaimCreationTypeExtension. Given the schema defaults, these are non-null where required, and null is preserved for TEE/localization so backend/feature flags can still decide behavior.If you anticipate adding more
ClaimCreationTypeApivariants later, you might add a default branch intoClaimCreationTypeto avoid future compile errors when extending the enum.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
generated/android/src/main/java/org/reclaimprotocol/inapp_sdk/Messages.ktis excluded by!**/generated/**generated/ios/Sources/ReclaimInAppSdk/Messages.his excluded by!**/generated/**generated/ios/Sources/ReclaimInAppSdk/Messages.mis excluded by!**/generated/**generated/ios/Sources/ReclaimInAppSdk/Messages.swiftis excluded by!**/generated/**
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)lib/reclaim_verifier_module.dart(4 hunks)lib/src/pigeon/messages.pigeon.dart(39 hunks)pigeon/schema.dart(3 hunks)
🔇 Additional comments (10)
CHANGELOG.md (1)
1-8: 0.25.0 entry aligns with the implemented API changesThe new changelog bullets correctly describe the added verification options (locale/useTeeOperator), TEE+MPC support, and script-facing APIs exposed in this PR; nothing blocking here.
lib/reclaim_verifier_module.dart (1)
443-463: New feature overrides are wired through cleanlyThe additional fields on
ClientFeatureOverrides(useTEE, interceptorOptions, timeout/debounce intervals, teeUrls) are passed straight intoReclaimFeatureFlagData, preserving nullability so feature flags/backend can decide behavior. This keeps the Dart-side thin and looks consistent with the existing overrides pattern.pigeon/schema.dart (2)
80-134: ClientFeatureOverrides surface extension is coherentThe added fields (useTEE, interceptorOptions, various timeout/debounce intervals, teeUrls) are all nullable and documented, and their declaration order matches the generated
ClientFeatureOverridesin messages.pigeon.dart, avoiding index-mismatch issues at the channel boundary.
193-238: ReclaimApiVerificationOptions: locale and TEE flag are well specifiedAdding
localeanduseTeeOperatorwith clear semantics (forced locale vs backend-chosen, TEE+MPC vs proxy attestor vs feature-flag default) cleanly extends the API without breaking existing callers, since the new fields are nullable and default to backend behavior when omitted.lib/src/pigeon/messages.pigeon.dart (6)
29-41: Deep equality helper looks correct but relies on Dart 3 features
_deepEqualshandlesListandMaprecursively and falls back to==otherwise, which is appropriate for these Pigeon models. It usesList.indexedwith record destructuring, so ensure your package’senvironment.sdkconstraint is set high enough (Dart 3.x) that all consumers can compile this.Please double-check your
pubspec.yamlhas a Dart SDK lower bound compatible withList.indexedand record patterns.
278-380: ClientFeatureOverrides encode/decode ordering is consistentThe
_toList()anddecode()implementations agree on field order (0–16), covering the new fields (attestorBrowserRpcUrlthroughteeUrls) without gaps or mismatches. This should serialize cleanly across the Flutter ↔ host boundary.
571-660: ReclaimApiVerificationOptions encoding covers locale and useTeeOperator correctlyThe new
localeanduseTeeOperatorfields are appended to_toList()and read back at indices 5 and 6 indecode(), matching the constructor order. Equality/hashCode now also account for these fields via_toList(), so they participate properly in comparisons and collections.
799-901: Codec updates correctly register all custom typesThe
_PigeonCodecwrites each custom enum/data class with a distinct type tag (129–144) and delegates toencode()/decode(). The correspondingreadValueOfTypebranches line up with these tags, so there’s no obvious tag collision or missing case.
903-1216: ReclaimModuleApi.setUp: stronger argument checks and error wrappingThe regenerated
setUpadds per-channel asserts for non-null messages/arguments and consistently wraps non-PlatformExceptionerrors into a genericPlatformException(code: 'error', message: e.toString()). This should make host-module integration failures surface more clearly without changing the happy path.
1219-1468: Host API wrappers match the updated protocol
ReclaimHostOverridesApiandReclaimHostVerificationApinow construct channel names withmessageChannelSuffix, send positional argument lists in the expected order, and validate replies (null/error/non-null) consistently. The signatures and argument ordering align with the schema, so these wrappers should interoperate correctly with the native side.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/build_ios.sh (1)
69-82: Remove commented-out code.The fat binary splitting logic (lines 69–82) is commented out and appears to be experimental or deprecated. Remove it to reduce cognitive load and avoid confusion during future maintenance.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
scripts/build_ios.sh (1)
98-105: Remove dead code: ONLY_RELEASE_TARGETS is hardcoded to true.The
ONLY_RELEASE_TARGETSvariable is hardcoded totrue, making the conditional at lines 101–102 dead code that will never execute. The recursive glob pattern (**) in line 102 also won't work withoutshopt -s globstarenabled.Either remove the dead branch for clarity or enable the flag if you intend to support nested directories in the future.
+# Enable recursive glob if needed +# shopt -s globstar ONLY_RELEASE_TARGETS=true -FRAMEWORK_PATTERN="" if [ "$ONLY_RELEASE_TARGETS" != "true" ]; then FRAMEWORK_PATTERN="build/ios/ReclaimXCFrameworks/**/*.framework" else FRAMEWORK_PATTERN="build/ios/ReclaimXCFrameworks/*.framework" fiOr, simplify by removing the dead branch:
ONLY_RELEASE_TARGETS=true -FRAMEWORK_PATTERN="" -if [ "$ONLY_RELEASE_TARGETS" != "true" ]; then - FRAMEWORK_PATTERN="build/ios/ReclaimXCFrameworks/**/*.framework" -else - FRAMEWORK_PATTERN="build/ios/ReclaimXCFrameworks/*.framework" -fi +FRAMEWORK_PATTERN="build/ios/ReclaimXCFrameworks/*.framework"
| # Example usage: | ||
| # create_xcframework objective_c | ||
| create_xcframework() { | ||
| local somepackage="$1" | ||
|
|
||
| if [ -z "$somepackage" ]; then | ||
| echo "Usage: create_xcframework <package_name>" | ||
| return 1 | ||
| fi | ||
|
|
||
| mkdir -p iphoneos | ||
| mkdir -p iphonesimulator | ||
|
|
||
| rm -rf "$somepackage.framework/_CodeSignature" | ||
| cp -R "$somepackage.framework/" "iphoneos/$somepackage.framework" | ||
| cp -R "$somepackage.framework/" "iphonesimulator/$somepackage.framework" | ||
|
|
||
| echo "Original framework architectures:" | ||
| xcrun lipo -info "$somepackage.framework/$somepackage" | ||
|
|
||
| # Create iphoneos version (remove x86_64, keep arm64) | ||
| xcrun lipo -remove x86_64 "./iphoneos/$somepackage.framework/$somepackage" -o "./iphoneos/$somepackage.framework/$somepackage" | ||
| vtool -set-build-version 2 14 26.1 -output "./iphoneos/$somepackage.framework/$somepackage" "./iphoneos/$somepackage.framework/$somepackage" | ||
| vtool -remove-build-version 7 -output "./iphoneos/$somepackage.framework/$somepackage" "./iphoneos/$somepackage.framework/$somepackage" | ||
|
|
||
| echo "iphoneos framework architectures:" | ||
| xcrun lipo -info "iphoneos/$somepackage.framework/$somepackage" | ||
|
|
||
| # Create iphonesimulator version (remove arm64, keep x86_64) | ||
| xcrun lipo -remove arm64 "./iphonesimulator/$somepackage.framework/$somepackage" -o "./iphonesimulator/$somepackage.framework/$somepackage" | ||
|
|
||
| echo "iphonesimulator framework architectures:" | ||
| xcrun lipo -info "iphonesimulator/$somepackage.framework/$somepackage" | ||
|
|
||
| # Create XCFramework | ||
| xcodebuild -create-xcframework \ | ||
| -framework "iphoneos/$somepackage.framework/" \ | ||
| -framework "iphonesimulator/$somepackage.framework/" \ | ||
| -output "$somepackage.xcframework" | ||
|
|
||
| rm -rf ./iphonesimulator; | ||
| rm -rf ./iphoneos; | ||
| } |
There was a problem hiding this comment.
Add error handling and prevent destructive operations on the original framework.
The create_xcframework function has several critical issues:
- Destructive operation on original: Line 64 removes the code signature from the original framework, and the function modifies it in-place before copying. If the function fails partway through, the original framework is corrupted.
- No error handling: Lines 69, 72–74, 80 and 86–89 execute binary manipulation commands (
lipo,vtool,xcodebuild) without checking for success. If any command fails, the script continues with broken binaries. - No validation: The function assumes
$somepackage.framework/$somepackageexists without checking.
Refactor to:
- Work on copies only; do not modify the original framework.
- Check that the binary exists before calling
lipo/vtool. - Validate that each command succeeds before continuing.
create_xcframework() {
local somepackage="$1"
if [ -z "$somepackage" ]; then
echo "Usage: create_xcframework <package_name>"
return 1
fi
+ # Validate source framework exists
+ if [ ! -d "$somepackage.framework" ]; then
+ echo "❌ Error: Framework not found: $somepackage.framework"
+ return 1
+ fi
+
+ if [ ! -f "$somepackage.framework/$somepackage" ]; then
+ echo "❌ Error: Binary not found: $somepackage.framework/$somepackage"
+ return 1
+ fi
mkdir -p iphoneos
mkdir -p iphonesimulator
- rm -rf "$somepackage.framework/_CodeSignature"
- cp -R "$somepackage.framework/" "iphoneos/$somepackage.framework"
- cp -R "$somepackage.framework/" "iphonesimulator/$somepackage.framework"
+ # Work on copies only; preserve the original
+ cp -R "$somepackage.framework/" "iphoneos/$somepackage.framework"
+ cp -R "$somepackage.framework/" "iphonesimulator/$somepackage.framework"
+
+ rm -rf "iphoneos/$somepackage.framework/_CodeSignature"
+ rm -rf "iphonesimulator/$somepackage.framework/_CodeSignature"
echo "Original framework architectures:"
- xcrun lipo -info "$somepackage.framework/$somepackage"
+ xcrun lipo -info "$somepackage.framework/$somepackage" || {
+ echo "❌ Error: Failed to extract architecture info"
+ return 1
+ }
# Create iphoneos version (remove x86_64, keep arm64)
- xcrun lipo -remove x86_64 "./iphoneos/$somepackage.framework/$somepackage" -o "./iphoneos/$somepackage.framework/$somepackage"
- vtool -set-build-version 2 14 26.1 -output "./iphoneos/$somepackage.framework/$somepackage" "./iphoneos/$somepackage.framework/$somepackage"
- vtool -remove-build-version 7 -output "./iphoneos/$somepackage.framework/$somepackage" "./iphoneos/$somepackage.framework/$somepackage"
+ xcrun lipo -remove x86_64 "./iphoneos/$somepackage.framework/$somepackage" -o "./iphoneos/$somepackage.framework/$somepackage" || {
+ echo "❌ Error: Failed to slice iphoneos framework"
+ return 1
+ }
+ vtool -set-build-version 2 14 26.1 -output "./iphoneos/$somepackage.framework/$somepackage" "./iphoneos/$somepackage.framework/$somepackage" || {
+ echo "❌ Error: Failed to set iphoneos build version"
+ return 1
+ }
+ vtool -remove-build-version 7 -output "./iphoneos/$somepackage.framework/$somepackage" "./iphoneos/$somepackage.framework/$somepackage" || {
+ echo "❌ Error: Failed to remove build version from iphoneos"
+ return 1
+ }
echo "iphoneos framework architectures:"
xcrun lipo -info "iphoneos/$somepackage.framework/$somepackage"
# Create iphonesimulator version (remove arm64, keep x86_64)
- xcrun lipo -remove arm64 "./iphonesimulator/$somepackage.framework/$somepackage" -o "./iphonesimulator/$somepackage.framework/$somepackage"
+ xcrun lipo -remove arm64 "./iphonesimulator/$somepackage.framework/$somepackage" -o "./iphonesimulator/$somepackage.framework/$somepackage" || {
+ echo "❌ Error: Failed to slice iphonesimulator framework"
+ return 1
+ }
echo "iphonesimulator framework architectures:"
xcrun lipo -info "iphonesimulator/$somepackage.framework/$somepackage"
# Create XCFramework
xcodebuild -create-xcframework \
-framework "iphoneos/$somepackage.framework/" \
-framework "iphonesimulator/$somepackage.framework/" \
-output "$somepackage.xcframework"
+ if [ $? -ne 0 ]; then
+ echo "❌ Error: Failed to create XCFramework"
+ return 1
+ fi
rm -rf ./iphonesimulator;
rm -rf ./iphoneos;
}🤖 Prompt for AI Agents
In scripts/build_ios.sh around lines 51–93, the function currently mutates the
original framework, lacks existence checks, and doesn't validate command
success; refactor so the script first verifies "$somepackage.framework" and its
binary exist, then work only on copies (create temp dirs and copy the framework
there rather than removing _CodeSignature from the original), run each tool
(lipo, vtool, xcodebuild) against the copies, check each command's exit status
and abort with a clear error message if any step fails, and ensure proper
cleanup of temp directories on both success and failure to avoid leaving partial
artifacts.
| for framework_path in $FRAMEWORK_PATTERN; do | ||
| echo "Trying to make XCframework for $framework_path" | ||
| if [ -d "$framework_path" ]; then | ||
| framework_name=$(basename $framework_path .framework) | ||
|
|
||
| echo "📦 Creating xcframework for $framework_name" | ||
|
|
||
| cd "$(dirname $framework_path)" | ||
| create_xcframework $framework_name | ||
| cd $project_dir; | ||
|
|
||
| rm -rf $framework_path | ||
| fi | ||
| done |
There was a problem hiding this comment.
Quote all variables to prevent word splitting and globbing in critical contexts.
Unquoted variables in the loop create security and correctness issues, especially in the rm -rf command:
- Line 114:
$framework_pathinbasenameshould be quoted to handle paths with spaces. - Line 118:
$(dirname $framework_path)should quote the argument. - Line 120:
$project_dirincdshould be quoted. - Line 122: Critical:
rm -rf $framework_pathwith an unquoted variable can expand to multiple paths or globs, causing unintended deletions.
-for framework_path in $FRAMEWORK_PATTERN; do
+for framework_path in $FRAMEWORK_PATTERN; do
echo "Trying to make XCframework for $framework_path"
if [ -d "$framework_path" ]; then
- framework_name=$(basename $framework_path .framework)
+ framework_name=$(basename "$framework_path" .framework)
echo "📦 Creating xcframework for $framework_name"
- cd "$(dirname $framework_path)"
+ cd "$(dirname "$framework_path")" || {
+ echo "❌ Error: Failed to change directory to $(dirname "$framework_path")"
+ continue
+ }
- create_xcframework $framework_name
+ create_xcframework "$framework_name"
- cd $project_dir;
+ cd "$project_dir" || {
+ echo "❌ Error: Failed to return to project directory"
+ exit 1
+ }
- rm -rf $framework_path
+ rm -rf "$framework_path"
fi
done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for framework_path in $FRAMEWORK_PATTERN; do | |
| echo "Trying to make XCframework for $framework_path" | |
| if [ -d "$framework_path" ]; then | |
| framework_name=$(basename $framework_path .framework) | |
| echo "📦 Creating xcframework for $framework_name" | |
| cd "$(dirname $framework_path)" | |
| create_xcframework $framework_name | |
| cd $project_dir; | |
| rm -rf $framework_path | |
| fi | |
| done | |
| for framework_path in $FRAMEWORK_PATTERN; do | |
| echo "Trying to make XCframework for $framework_path" | |
| if [ -d "$framework_path" ]; then | |
| framework_name=$(basename "$framework_path" .framework) | |
| echo "📦 Creating xcframework for $framework_name" | |
| cd "$(dirname "$framework_path")" || { | |
| echo "❌ Error: Failed to change directory to $(dirname "$framework_path")" | |
| continue | |
| } | |
| create_xcframework "$framework_name" | |
| cd "$project_dir" || { | |
| echo "❌ Error: Failed to return to project directory" | |
| exit 1 | |
| } | |
| rm -rf "$framework_path" | |
| fi | |
| done |
🤖 Prompt for AI Agents
In scripts/build_ios.sh around lines 111-124, unquoted variables risk
word-splitting and glob expansion; update the loop to quote all variable
expansions: use "$(basename "$framework_path" .framework)" when computing
framework_name, use cd "$(dirname "$framework_path")" when changing dirs, use cd
"$project_dir" when returning, and make the deletion safe by using rm -rf --
"$framework_path" (and quote any other expansions like "$FRAMEWORK_PATTERN" if
used elsewhere) so paths with spaces or leading dashes are handled correctly.
Summary by CodeRabbit
New Features
Chore
✏️ Tip: You can customize this high-level summary in your review settings.