WalletConnect / WalletConnectFlutterV2

WalletConnect v2 client made in Dart for Flutter.
https://pub.dev/packages/walletconnect_flutter_v2
Apache License 2.0
101 stars 52 forks source link

Add access to approve session properties #271

Closed hmendez-figure closed 2 months ago

hmendez-figure commented 4 months ago

Description

Allows full access to SignEngine.approveSession (specifically sessionProperties) params from interfaces.

dApp and wallet are able to send optional sessionProperties via Proposal and Settlement messages according to spec. Currently the dApp client exposes this via connect() but wallet client does not in approveSession even though SignEngine.approveSession in fact does implement it. Though the interface (ISignEngineWallet) does not expose it.

I compared this to the JS sign client and it also exposes it here

Resolves # (issue)

How Has This Been Tested?

Smoke Test

Due Dilligence

quetool commented 4 months ago

Hello @hmendez-figure, thanks for opening! I'll check this PR as soon as I can, in the meantime could you add in the description and explanation on why is this needed? Thanks!

sonarcloud[bot] commented 4 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

VigM-Figure commented 2 months ago

Hi @quetool, was there anything else needed or any concerns holding up this pull request? Our organization has taken to forking the repo to solve the problem about not having access to sessionProperties but would ideally like to revert and stay on the main branch/repo if possible. Thanks!

quetool commented 2 months ago

Hello @VigM-Figure! Actually thanks for the ping! I will take care of this soon! Thanks!

quetool commented 2 months ago

Hello @hmendez-figure, do you mind trying test on your end and fix then if any? Thanks!

quetool commented 2 months ago

Never mind! I will just merge it and take care of it on my end! Thanks @hmendez-figure @VigM-Figure for this and sorry for the huge delay! It will be deployed on the next release! 👌