contracts: wrapped resources #1704
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Preparing PR description... |
|
Preparing review... |
![mentatbot[bot]](https://avatars.githubusercontent.com/in/837875?s=60&v=4){kind=small}
There was a problem hiding this comment.
The implementation of the resource bridge system is well-structured and covers the main functionalities required for depositing and withdrawing ERC20 tokens. The code includes proper checks for pausing deposits/withdrawals, whitelisting tokens, and fee calculations. However, there are a few areas where the code could be improved:
- Additional checks for zero-value transactions could be added to prevent potential issues.
- Error handling or logging for precision loss during token-resource conversions could be beneficial.
- The fee calculation logic is duplicated and could be abstracted into a separate function.
- Some error messages could be more descriptive to aid in debugging.
Overall, the implementation is solid, but these small improvements could enhance its robustness and maintainability.
Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!
|
|
||
| // take fees from deposit | ||
| let total_fees = InternalBridgeImpl::send_fees_on_deposit(world, token, client_fee_recipient, amount); | ||
| assert!(total_fees.is_non_zero(), "Bridge: deposit amount too small"); |
There was a problem hiding this comment.
Consider adding a check to ensure the amount is greater than zero to prevent potential issues with zero-value deposits.
contracts/src/systems/resources/contracts/resource_bridge_systems.cairo
Outdated
Show resolved
Hide resolved
| return relative_amount; | ||
| } | ||
|
|
||
| fn send_fees_on_deposit( |
There was a problem hiding this comment.
Consider adding error handling or logging for cases where the conversion might result in a loss of precision.
| let fee_split_config = get!(world, WORLD_CONFIG_ID, ResourceBridgeFeeSplitConfig); | ||
| let velords_fee_amount = (amount * fee_split_config.velords_fee_on_dpt_percent.into()) | ||
| / PercentageValueImpl::_100().into(); | ||
| let season_pool_fee_amount = (amount * fee_split_config.season_pool_fee_on_dpt_percent.into()) |
There was a problem hiding this comment.
Similar to the previous comment, consider adding error handling or logging for potential precision loss during conversion.
| / PercentageValueImpl::_100().into(); | ||
|
|
There was a problem hiding this comment.
The fee calculation logic is repeated in both deposit and withdrawal functions. Consider abstracting this into a separate function to reduce code duplication.
| / PercentageValueImpl::_100().into(); | |
| let client_fee_amount = Self::calculate_fee(amount, fee_split_config.client_fee_on_dpt_percent); |
contracts/src/systems/resources/contracts/resource_bridge_systems.cairo
Outdated
Show resolved
Hide resolved
resolves #1150
ERC20token can be deposited and withdrawn for a fee