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

feat: thorchain lp disable smart contract deposits #6177

Merged
merged 1 commit into from
Feb 19, 2024

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Feb 12, 2024

Description

This PR introduces smart contract checks for THORChain LP deposits.

Pull Request Type

  • 🐛 Bug fix (Non-breaking Change: Fixes an issue)
  • 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

closes #6107

Risk

High Risk PRs Require 2 approvals

Low from a theoretical perspective, high as we should ensure that

  1. this works (obviously) and doesn't let smart contract wallets proceed
  2. this doesn't block non-smart contract wallets from continuing
  3. there is no flow other than LP deposits affected by this

What protocols, transaction types or contract interactions might be affected by this PR?

THORChain smart contracts

Testing

  • Smart contract wallets are blocked from depositing into LP
  • Other wallets are happy with LP depositing

Engineering

  • 👆

Operations

  • 👆

Screenshots (if applicable)

  • SC wallet

@gomesalexandre gomesalexandre marked this pull request as ready for review February 12, 2024 15:41
@gomesalexandre gomesalexandre requested a review from a team as a code owner February 12, 2024 15:41
@0xApotheosis 0xApotheosis self-assigned this Feb 12, 2024
Copy link
Contributor

@0xApotheosis 0xApotheosis left a comment

Choose a reason for hiding this comment

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

✅ Non-smart contracts can still interact with LP pools as expected

Screenshot 2024-02-13 at 9 45 18 am Screenshot 2024-02-13 at 9 46 10 am

✅ Smart contracts are blocked from doing so

Screenshot 2024-02-13 at 9 49 11 am

@gomesalexandre gomesalexandre force-pushed the feat_query_key_factory_shenanigans branch from 081e94a to 1ce9785 Compare February 13, 2024 00:08
@gomesalexandre gomesalexandre force-pushed the thor_lp_smart_deposits branch 3 times, most recently from 2df5d21 to 2cbf4da Compare February 14, 2024 00:11
Base automatically changed from feat_query_key_factory_shenanigans to develop February 16, 2024 08:56
@0xApotheosis 0xApotheosis merged commit 9792205 into develop Feb 19, 2024
4 checks passed
@0xApotheosis 0xApotheosis deleted the thor_lp_smart_deposits branch February 19, 2024 02:21
@0xApotheosis 0xApotheosis mentioned this pull request Feb 20, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

THORChain LP smart contract detection
3 participants