Skip to content

Conversation

@tbwebb22
Copy link

@tbwebb22 tbwebb22 commented Oct 21, 2025

Adds swapsProcessed and totalSwapsProcessed as return vars to finalizePendingSwaps

Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
@tbwebb22 tbwebb22 force-pushed the taylor/finalize-swap-return-data branch from cdfbaea to 4a6817b Compare October 21, 2025 17:19
@tbwebb22 tbwebb22 marked this pull request as ready for review October 21, 2025 17:34
function finalizePendingSwaps(
address finalToken,
uint256 maxToProcess
) external returns (uint256 swapsProcessed, uint256 totalSwapsProcessed) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There seems to be 2 types of terminology we use in this function:
the name of the fn is finalize.. and the name of internal var is processed

I'm leaning that we stick with the finalizePendingSwaps name for the fn, and change return var names to ..Finalized. We could also change the local processed fn name to finalized (although maybe not because it increases the diff from the "audit commit")

Also, from the POV of the bot who's calling this, the bot may care about both the number of processed and the amount (in tokens) of them. For example, the bot may want to finalize only if the total amount in simulated finalization is e.g. more than $1000 or something.

Also, from the POV of the bot totalSwapsProcessed doesn't tell it much (current-decision-wise). E.g. it probably won't have any logic branches based on totalSwapsProcessed(I think?)

What could be useful to return is something like totalPendingSwapsRemaning.
Then a bot e.g. could reason:

if the totalAmountFinalized is little, but if it brings our queue down to zero, then I'll finalize anyway.

Wdyt?

Copy link
Author

Choose a reason for hiding this comment

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

I'm in favor of changing return vars and local vars to finalize.. for consistency. Doesn't change the audit diff all that much as its just renamings

Agree that totalSwapsProcessed isn't that useful for the bot. But swapAmountsFinalized and pendingSwapsRemaining definitely would be. I can update to this

Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
@tbwebb22 tbwebb22 force-pushed the taylor/finalize-swap-return-data branch from 60ecb9f to c03a08c Compare October 21, 2025 18:55
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
@grasphoper grasphoper merged commit 4e8e1a2 into audit-oct-20 Oct 22, 2025
9 of 10 checks passed
@grasphoper grasphoper deleted the taylor/finalize-swap-return-data branch October 22, 2025 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants