Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing SDK config params to injected provider #1436

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

fan-zhang-sv
Copy link
Contributor

@fan-zhang-sv fan-zhang-sv commented Oct 23, 2024

Summary

extended the setAppInfo pattern to RN's in app browser provider.
This is used to block EOA account to connect to smartWalletOnly app, and also enables adding data suffix in future RN smart wallet account

we are passing flattened params, including metadata and preference, this is for maximizing compatibility across App SDK version in case of future App SDK config structure change

How did you test your changes?

Manually, demo attached in RN's PR

Demo: noop on current extension

Screen.Recording.2024-10-25.at.1.35.03.PM.mov

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Oct 23, 2024

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@fan-zhang-sv fan-zhang-sv changed the title ethereum.setAppParams Passing SDK config params to injected provider Oct 23, 2024
if (preference.options !== 'smartWalletOnly') {
const extension = getCoinbaseInjectedLegacyProvider();
if (extension) {
const { appName, appLogoUrl, appChainIds } = metadata;
extension.setAppInfo?.(appName, appLogoUrl, appChainIds);
extension.setAppInfo?.(appName, appLogoUrl, appChainIds, preference);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding it for extension too, just in case we need to use it, no harm even if we dont

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you test this on the extension side? Maybe include a video in the PR description?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added demo!

@fan-zhang-sv fan-zhang-sv merged commit 62b243c into master Oct 25, 2024
8 checks passed
@fan-zhang-sv fan-zhang-sv deleted the felix/sdk-to-pass-preference branch October 25, 2024 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants