Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an iframe-based communication path for dApps embedded inside the Startale app, avoiding popup-based flows by introducing an ICommunicator abstraction and an IframeCommunicator implementation.
Changes:
- Introduces
ICommunicatorand updates existing popupCommunicator/Signer/BaseAccountProviderto depend on the interface. - Adds iframe detection (
shouldUseIframeMode) and wirescreateStartaleAccountSDKto useIframeCommunicatorwhen embedded. - Adds/updates tests for iframe mode and exports the new communicator types from the SDK entrypoint.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/app-sdk/src/sign/app-sdk/Signer.ts | Switches communicator dependency from concrete Communicator to ICommunicator. |
| packages/app-sdk/src/interface/builder/core/createStartaleAccountSDK.ts | Chooses iframe mode via shouldUseIframeMode; constructs IframeCommunicator + passes into provider. |
| packages/app-sdk/src/interface/builder/core/createStartaleAccountSDK.test.ts | Adds test coverage for iframe-mode provider creation and COOP check behavior. |
| packages/app-sdk/src/interface/builder/core/BaseAccountProvider.ts | Allows injecting an ICommunicator (iframe or popup) into the provider. |
| packages/app-sdk/src/index.ts | Exposes ICommunicator type and IframeCommunicator export. |
| packages/app-sdk/src/core/error/serialize.ts | Changes serialized error docUrl to an empty string. |
| packages/app-sdk/src/core/communicator/iframeUtils.ts | Adds iframe detection + allowed-origin logic. |
| packages/app-sdk/src/core/communicator/IframeCommunicator.ts | Adds iframe postMessage-based communicator and handshake logic. |
| packages/app-sdk/src/core/communicator/IframeCommunicator.test.ts | Adds tests for iframe communicator behavior (constructor/onMessage/handshake/etc). |
| packages/app-sdk/src/core/communicator/ICommunicator.ts | Defines the shared communicator interface. |
| packages/app-sdk/src/core/communicator/Communicator.ts | Implements ICommunicator for the existing popup-based communicator. |
| packages/app-sdk/package.json | Bumps SDK version to 1.3.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JGJP
reviewed
Mar 4, 2026
JGJP
reviewed
Mar 13, 2026
JGJP
approved these changes
Mar 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Create another route for communication with dApps hosted inside Startale app. For those dApps no popup window will be displayedMain reason is for doing this is to avoid adding complex code to Startale app which includes adding reference Google comlink and some hardly traceable and understandable logic as implemented here https://github.com/StartaleGroup/superapp/tree/miniapp-pocI needed to rethink the whole concept because I finally understood how Farcaster host for mini apps works and because on some requirements from the PM.
The requirements are
Farcaster injects EIP 1193 provider to iframe, so a hosted mini app can use it. Which means mini app devs can use injected provider with Farcaster connector. If we want them to use our SDK we need to support Farcaster in our SDK.
The approach I initially created forced devs to use SDK for their mini apps.
How did you test your changes?
Hoseted a miniapp using the current SDK inside Strtale app.