fix: on-chain update prices return excess send mode#626
Conversation
|
👋 vicentevieytes, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR adjusts TON fee/cost parameters and fixes how the FeeQuoter contract returns excess TON after UpdatePrices, so excess value is sent back without risking draining the contract’s existing balance.
Changes:
- Bumped default OCR ContractTransmitter TON cost constants for commit (price-only / price+root).
- Increased on-chain
FeeQuoter_Costs.updatePrices()value requirement from 0.01 TON to 0.02 TON. - Updated FeeQuoter’s excess-return mechanism to reserve the original balance and send excess using
SEND_MODE_CARRY_ALL_BALANCE.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/ccip/ocr/config.go | Updates default TON cost constants used by the OCR contract transmitter. |
| contracts/contracts/ccip/fee_quoter/messages.tolk | Increases the declared TON cost for updatePrices(). |
| contracts/contracts/ccip/fee_quoter/contract.tolk | Changes excess return to reserve original balance and carry all remaining balance to the refund recipient. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
patricios-space
left a comment
There was a problem hiding this comment.
Looks good, but if you plan to run an in-place upgrade, you'll need a migrate function.
ea68bda to
0ddf173
Compare
|
|
||
| @method_id(1000) | ||
| fun migrate(storage: cell, version: slice): cell { throw Upgradeable_Error.VersionMismatch; } | ||
| fun migrate(storage: cell, version: slice): cell { |
There was a problem hiding this comment.
@patricios-space so here I should check that a PREVIOUS_VERSION matches the value passed in the version parameter right?
There was a problem hiding this comment.
Yes! In other contracts we added const CONTRACT_VERSION_PREV = "1.6.0"; at the top of the contract.tolk file and then we assert version.bitEquals(CONTRACT_VERSION_PREV)
… vv/fix-update-prices-return-excess-send-mode
| }) | ||
|
|
||
| const finalBalance = (await blockchain.getContract(setup.bind.feeQuoter.address)).balance | ||
| expect(finalBalance).toEqual(initialBalance) |
There was a problem hiding this comment.
This is flaky as storagefee could change the balance. See
for the fix| }); | ||
| returnExcesses.send(SEND_MODE_CARRY_ALL_REMAINING_MESSAGE_VALUE); | ||
| reserveToncoinsOnBalance(0, RESERVE_MODE_INCREASE_BY_ORIGINAL_BALANCE); | ||
| returnExcesses.send(SEND_MODE_CARRY_ALL_BALANCE); |
There was a problem hiding this comment.
SEND_MODE_CARRY_ALL_REMAINING_MESSAGE_VALUE does not take into account the amount already spent for the event that is emited when processing this message. This means that a little more TON is sent than the amount is received to trigger the contract and the balance slowly drains over time.
Instead, we reserve an amount equal to the original contract balance and then try to send all of the possible TON. Because of the reserve, the contract will end up with exactly the same amount of TON as at the start and it will not drain.
No description provided.