-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: add WalletConnect integration for Ledger devices #720
base: master
Are you sure you want to change the base?
feat: add WalletConnect integration for Ledger devices #720
Conversation
- Add WalletConnect integration using @reown/appkit - Replace direct Ledger integration with WalletConnect - Update session management to support WalletConnect - Add Harmony Mainnet chain configuration Link to Devin run: https://app.devin.ai/sessions/9e403c268ac34438a5e04b2de9cc557e Requested by: Aaron Co-Authored-By: Aaron Li <[email protected]>
…gning # Conflicts: # frontend/yarn.lock
const { gasEstimate } = fee | ||
const { chain_id, rpc_url } = networkConfig | ||
|
||
const harmony = new Harmony(rpc_url, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect. Harmony
class is undefined. Please do not simply copy from old implementations. Instead, properly invoke correct contract calling methods from wagmi
|
||
switch (transactionData.type) { | ||
case "MsgSend": | ||
return harmony.transactions.signTransaction({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, this is incorrect because the instance harmony
would not exist and would not have the field transactions
}) | ||
|
||
case "MsgDelegate": | ||
return harmony.stakings.delegate({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
}) | ||
|
||
case "MsgUndelegate": | ||
return harmony.stakings.undelegate({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
}) | ||
|
||
case "MsgWithdrawDelegationReward": | ||
return harmony.stakings.collectRewards({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
- Replace Harmony class usage with wagmi's writeContract - Add proper error handling in WalletConnect component - Use correct contract ABI and address for staking operations - Implement gas estimation using wagmi methods Co-Authored-By: Aaron Li <[email protected]>
Co-Authored-By: Aaron Li <[email protected]>
# Conflicts: # frontend/src/components/common/TmSessionExisting.vue # frontend/yarn.lock
Co-Authored-By: Aaron Li <[email protected]>
Co-Authored-By: Aaron Li <[email protected]>
Co-Authored-By: Aaron Li <[email protected]>
Co-Authored-By: Aaron Li <[email protected]>
Co-Authored-By: Aaron Li <[email protected]>
Co-Authored-By: Aaron Li <[email protected]>
Co-Authored-By: Aaron Li <[email protected]>
Co-Authored-By: Aaron Li <[email protected]>
Co-Authored-By: Aaron Li <[email protected]>
Fixes #719
Problem:
Solution:
Testing:
Note: This implementation follows the new approach suggested in #719, using WalletConnect instead of modifying the existing Ledger integration.
Link to Devin run: https://app.devin.ai/sessions/9e403c268ac34438a5e04b2de9cc557e
Requested by: Aaron