Conversation
All other entry functions validate the pools fee tier, except for this function. This is likely because it doesn't make use of the fees. To make the code consistent (and auditing easier), we're now validating the fees in `sync_reserves` the same way.
There was a problem hiding this comment.
Pull request overview
This PR makes AMM instruction validation consistent by adding fee-tier validation to sync_reserves, aligning it with other AMM entrypoints that already enforce supported fee tiers.
Changes:
- Import
assert_supported_fee_tierintoamm/src/sync.rs. - Validate
pool_def_data.feesinsync_reservesviaassert_supported_fee_tier(...).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let pool_def_data = PoolDefinition::try_from(&pool.account.data) | ||
| .expect("Sync reserves: AMM Program expects a valid Pool Definition Account"); | ||
| assert_supported_fee_tier(pool_def_data.fees); |
There was a problem hiding this comment.
Agreed. The change is correct, but I recommend to add a small unit test for sync_reserves that sets PoolDefinition.fees = 2 and expects the shared unsupported-fee panic message, cuz the current sync tests do not have this new guard directly.
3esmit
left a comment
3esmit
left a comment
There was a problem hiding this comment.
This guard looks right and matches the validation already present in add/remove/swap. One small test gap: would be good to narrow amm_program unit test that mutates PoolDefinition.fees to an unsupported value and expects Fee tier must be one of 1, 5, 30, or 100 basis points from sync_reserves.
| let pool_def_data = PoolDefinition::try_from(&pool.account.data) | ||
| .expect("Sync reserves: AMM Program expects a valid Pool Definition Account"); | ||
| assert_supported_fee_tier(pool_def_data.fees); |
There was a problem hiding this comment.
Agreed. The change is correct, but I recommend to add a small unit test for sync_reserves that sets PoolDefinition.fees = 2 and expects the shared unsupported-fee panic message, cuz the current sync tests do not have this new guard directly.
All other entry functions validate the pools fee tier, except for this function. This is likely because it doesn't make use of the fees.
To make the code consistent (and auditing easier), we're now validating the fees in
sync_reservesthe same way.