-
Notifications
You must be signed in to change notification settings - Fork 1
fix: removed circular dependency, added better error handling, lp-contract related refactoring #94
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
Conversation
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.
Pull Request Overview
This PR refactors LP deposit and vault contract logic to remove circular dependencies and improve error handling for liquidity provision, while also cleaning up redundant code and updating tests.
- Refactored error handling in liquidity-deposit contracts to send rejection messages and commit state before reverting.
- Updated vault interfaces and vault implementations (TON and Jetton) to align with the new error signaling mechanism.
- Extended test coverage for incorrect liquidity provision scenarios and removed duplicate message definitions to break circular dependency.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sources/tests/liquidity-deposit.spec.ts | Added new test cases for incorrect liquidity provision and error handling validation. |
| sources/contracts/vaults/vault-interface.tact | Introduced new messages (PartHasBeenDeposited and RejectLiquidityPart) and updated interface functions. |
| sources/contracts/vaults/ton-vault.tact | Implemented handleRejectLP with state initialization using the ammPoolCode. |
| sources/contracts/vaults/jetton-vault.tact | Added override for handleRejectLP and updated message parameters with ammPoolCode. |
| sources/contracts/core/messages.tact | Removed duplicate definition of PartHasBeenDeposited to eliminate circular dependency. |
| sources/contracts/core/liquidity-deposit.tact | Refactored error handling on liquidity deposit with conditional commit and error messages; added helper to build ammPool data. |
| sources/contracts/core/amm-pool.tact | Inserted additional reserve validations and removed redundant commented-out TODOs. |
Comments suppressed due to low confidence (1)
sources/contracts/core/liquidity-deposit.tact:60
- [nitpick] In the left vault branch, the commit() call immediately precedes a require(false). Please verify that committing state before reverting is intentional and add an inline comment to clarify this error-handling strategy.
require(false, "LP Deposit: Left side cannot be filled again or with different amount");
Kaladin13
left a comment
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.
Please let's add test that get method fails if no liquidity
Also I wonder about tests for new removed dependency, but since the rest of the tests work I think it;s fine
Closes #70