-
Notifications
You must be signed in to change notification settings - Fork 43
Extend instance init to allow core and chain abstraction params for b… #142
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
base: develop
Are you sure you want to change the base?
Conversation
…oth wallet and app kit usage
@quetool Please help to review this PR, thank you! |
Hello! Thanks, I'll take a look ASAP |
Hello @ludowkm, sorry for the late response on this! Is still in my bucket list but sadly I just couldn't run the proper tests yet |
Also, can you elaborate a bit more or share some code? Cause I am able to instantiate and initialize both AppKit and WalletKit |
Hello @quetool , thank you for the response. There is a case that an app use both walletkit and appkit. Both require init() separately so when we call both init(), there is some issue which make both walletkit and appkit not work. final _core = ReownCore(
projectId: BuildConfig.instance.walletConnectProjectID,
);
final reOwnSignInstance =
ReownWalletKit.createReOwnSignInstance(core: _core, metadata: metadata);
_walletKit = ReownWalletKit(
core: _core, metadata: metadata, overrideReOwnSign: reOwnSignInstance);
_appKit = ReownAppKit(
core: _core, metadata: metadata, overrideReOwnSign: reOwnSignInstance);
await _walletKit.init(); |
Hello @ludowkm, here I set up a little example where I initialize both appkit and walletkit. Can you modify this file in order to repro your issue? import 'package:flutter/material.dart';
import 'package:reown_appkit/reown_appkit.dart';
import 'package:reown_walletkit/reown_walletkit.dart';
void main() {
runApp(const MyApp());
}
class MyApp extends StatelessWidget {
const MyApp({super.key});
@override
Widget build(BuildContext context) {
return ReownAppKitModalTheme(
child: MaterialApp(
title: 'appkit_test_dapp',
theme: ThemeData(
colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
useMaterial3: true,
),
home: const HomePage(),
),
);
}
}
class HomePage extends StatefulWidget {
const HomePage({super.key});
@override
State<HomePage> createState() => _HomePageState();
}
class _HomePageState extends State<HomePage> {
late ReownAppKitModal _appKitModal;
late ReownWalletKit _walletKit;
@override
void initState() {
super.initState();
_initializeAppKit();
_initializeWalletKit();
}
Future<void> _initializeAppKit() async {
try {
_appKitModal = ReownAppKitModal(
context: context,
logLevel: LogLevel.all,
projectId: '50f81661a58229027394e0a19e9db752',
metadata: const PairingMetadata(
name: 'appkit_test_dapp',
description: 'appkit_test_dapp description',
url: 'https://appkit_test_dapp.com',
icons: ['https://appkit_test_dapp.com/logo.png'],
redirect: Redirect(
native: 'appkit_test_dapp://',
),
),
);
_appKitModal.appKit!.core.addLogListener(_appKitCoreListener);
// More events at https://docs.reown.com/appkit/flutter/core/events
_appKitModal.onModalConnect.subscribe(_onModalConnect);
_appKitModal.onModalDisconnect.subscribe(_onModalDisconnect);
await _appKitModal.init();
setState(() {});
} catch (e) {
debugPrint('❌ $e');
}
}
Future<void> _initializeWalletKit() async {
try {
// Create the ReownWalletKit instance
_walletKit = ReownWalletKit(
core: ReownCore(
projectId: '50f81661a58229027394e0a19e9db752',
logLevel: LogLevel.all,
),
metadata: const PairingMetadata(
name: 'appkit_test_wallet',
description: 'appkit_test_wallet description',
url: 'https://appkit_test_wallet.com',
icons: ['https://appkit_test_wallet.com/logo.png'],
redirect: Redirect(
native: 'appkit_test_wallet://',
),
),
);
_walletKit.core.addLogListener(_walletKitCoreListener);
// Setup our accounts
_walletKit.registerAccount(
chainId: 'eip155:1',
accountAddress: '0xsa87dfs6gd45sa6d85f74s68a769d5f7g456',
);
await _walletKit.init();
setState(() {});
} catch (e) {
debugPrint('❌ $e');
}
}
void _walletKitCoreListener(String message) {
print(message);
}
void _appKitCoreListener(String message) {
print(message);
}
void _onModalConnect(ModalConnect? event) {
setState(() {});
}
void _onModalDisconnect(ModalDisconnect? event) {
setState(() {});
}
@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(
title: const Text('appkit_test_dapp'),
backgroundColor: Theme.of(context).colorScheme.inversePrimary,
),
body: Center(
child: Column(
children: [
AppKitModalNetworkSelectButton(
appKit: _appKitModal,
context: context,
),
AppKitModalConnectButton(
appKit: _appKitModal,
context: context,
),
Visibility(
visible: _appKitModal.isConnected,
child: Column(
children: [
AppKitModalAccountButton(
appKitModal: _appKitModal,
context: context,
),
AppKitModalAddressButton(
appKitModal: _appKitModal,
onTap: () {},
),
AppKitModalBalanceButton(
appKitModal: _appKitModal,
onTap: () {},
),
ValueListenableBuilder<String>(
valueListenable: _appKitModal.balanceNotifier,
builder: (_, balance, __) {
return Text('My balance: $balance');
},
),
],
),
)
],
),
),
);
}
} Versions used are:
|
Hello @quetool, with this implementation, we create 2 instances of ReownCore with 2 Relay connections to be established. I think it's not best practice, is it? |
Also when we re-launch app and get active sessions from appkit or walletkit, it returns all sessions from both appkit and walletkit causes duplicated, while in runtime each kit returns each sessions only. |
Can you share your implementation? I'd like to check it. |
You can run the above your code snippet, my implementation is almost same. |
Beside that issue, i'm wondering why do we need to create 2 ReownCore instances as well as inside AppKit and WalletKit also has each own ReownSign instance. I thought we can use one ReownCore and ReownSign for both WalletKit and AppKit, don't it? |
You may have a valid point, @ludowkm, allow me some time to re-evaluate the implications of this and I'll reach back! Thank you very much! |
Hello @ludowkm! Apologize for my very late response! I will go over this today and give you a final answer! Thanks for your patience! |
Hello @ludowkm, can I see how do you implement this change on app side? I mean, how do you instantiate both AppKit and WalletKit |
Hello @quetool , i instantiated both wallet kit and app kit like the below. The different things are using same core and reOwn instance, then init once time from wallet kit only. final _core = ReownCore(
projectId: BuildConfig.instance.walletConnectProjectID,
);
final reOwnSignInstance = ReownWalletKit.createReOwnSignInstance(
core: _core,
metadata: metadata,
);
_walletKit = ReownWalletKit(
core: _core,
metadata: metadata,
overrideReOwnSign: reOwnSignInstance,
);
_appKit = ReownAppKit(
core: _core,
metadata: metadata,
overrideReOwnSign: reOwnSignInstance,
);
await _walletKit.init(); |
Thanks! So you are using ReownAppKit instance and not ReownAppKitModal? Anyway I'm working on a solution for you as I'm not convinced about this kind of constructor: ReownAppKit(
{required this.core,
required this.metadata,
ReownSign? overrideReOwnSign}) {
reOwnSign = overrideReOwnSign ??
createReOwnSignInstance(core: core, metadata: metadata);
} This constructor requires a core instance but you are also overriding reownSign instance with an instance that also contains a core instance, which happens to be the same as the one passed in the constructor. Looks confusing and potentially problematic. Also, since you are using both sides of the sign engine (appkit and walletkit) I can suggest you using just a ReownSignClient instance. Specially if you are not using the Chain Abstraction functionality in WalletKit. final reownSignClient = ReownSignClient(
core: ReownCore(
projectId: '50f81661a58229027394e0a19e9db752',
),
metadata: const PairingMetadata(
name: 'appkit_test_dapp',
description: 'appkit_test_dapp description',
url: 'https://appkit_test_dapp.com',
icons: ['https://appkit_test_dapp.com/logo.png'],
redirect: Redirect(
native: 'appkit_test_dapp://',
),
),
);
await reownSignClient.init(); It is essentially the same, AppKit and WalletKit are just higher level of abstraction of SignEngine (which is wrapped in SignClient) I'm a looking on a solution for your case anyway! |
Hey @quetool , you are totally right, i should use ReownSignClient because i did not use functionalities of WalletKit and AppKit. My bad when switch from plugin https://github.com/wakumo/flutter-wallet-connect-v2 to Reown native dart lib. Thank you for your reply and the solution! So this PR is unnecessary, we can close it! |
No worries @ludowkm, I'd like to keep it opened as I'd like to evaluate the possibility of having both AppKit and WalletKit to work together in full capacities. |
…oth wallet and app kit usage
Description
There are some projects using both WalletKit and AppKit but the current implementation of constructor just allow to use only one of that kit.
So now i extend it to support optional params, static method to create required instance, then we are able to init both WalletKit and AppKit to use in single project.