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

[WIP] feature: bbn-wallet-connect native lib #409

Closed
wants to merge 1 commit into from

Conversation

gbarkhatov
Copy link
Contributor

@gbarkhatov gbarkhatov commented Nov 28, 2024

The purpose of this PR is to have a working connection with the switch to a native wallet connector via @babylonlabs-io/bbn-wallet-connect

This PR is a combination of:

  1. feat: add bbn provider #404
  2. feat: BBN wallet connect #396
  3. current main changes
  4. some temp fixes to make the staking process work

Issues could be solved across different repos:

Prioritized:

  • Ordinals <- Can be a part of different PR

  • Build

    • From main
    • Install lib
    • investigate the context
  • Wallets disconnection process

  • Connection can be a part of later PRs

  • Scroll locker is minor

  • Tomo wallet connector is big, should be a part of later PRs

  • Connection persistance we're not touching right now

Problems, in terms of priority:

  1. Ordinals / Inscriptions will throw an error / not working
  2. Connection process is wrong:
  1. No TOMO wallet connector integrated
  2. When disconnected, the connect button is not clickable https://i.imgur.com/MyA15C7.png
  3. Build process keeps failing, both on local and Github CI https://i.imgur.com/Ch22gUU.png
  4. Scroll Locker is not working. All the connection modals allow user to scroll. This is not the case for a regular app modals. For example, Preview modal is locking the scroll https://i.imgur.com/fV7ugQr.png
  5. Connection is not persisted (on refresh need to reconnect once again) <- minor, can be extracted to an issue in https://github.com/babylonlabs-io/bbn-wallet-connect

Overall the connection works, I was able to stake using OKX + Keplr

@gbarkhatov gbarkhatov requested a review from totraev November 28, 2024 18:58
This was referenced Nov 28, 2024
@gbarkhatov gbarkhatov force-pushed the feature/bbn-wallet-connect branch from 006acd8 to 928a429 Compare November 28, 2024 19:40
@totraev
Copy link
Contributor

totraev commented Nov 29, 2024

  1. Implemented according current requirements, pls check here: https://www.figma.com/design/05UW9XBXoFWcKEEIB1ZetV/Design-File?node-id=303-21335&node-type=canvas&m=dev
  2. Don't worry about this for now. We need make core-ui a peer dependency of bbn-wallet-connect to fix problem with Scroll Locker

@@ -27,6 +27,7 @@
"dependencies": {
"@babylonlabs-io/babylon-proto-ts": "0.0.3-canary.3",
"@babylonlabs-io/bbn-core-ui": "^0.2.0",
"@babylonlabs-io/bbn-wallet-connect": "^0.0.18",
Copy link
Collaborator

Choose a reason for hiding this comment

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

should the tomo dependency be removed from this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jrwbabylonlab Tomo wallet connect is not currently implemented, it requires additional abstraction

@totraev Should Tomo wallet connect be a dependency and the part of the simple-staking or bbn-wallet-connect?

Copy link
Collaborator

Choose a reason for hiding this comment

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

i'm getting confused. we are going to use the bbn-wallet-connect. why tomo is still here?

@gbarkhatov
Copy link
Contributor Author

gbarkhatov commented Nov 29, 2024

Implemented according current requirements, pls check here:

@totraev according to the design the connection flow assumption in the notes are correct - Once clicked BTC should be "connected" with the address and all of that only within the modal, not within the app

@totraev
Copy link
Contributor

totraev commented Nov 30, 2024

@gbarkhatov can be closed in favor of new PR #413

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants