In a comment on the sponsor's public Discord, it was confirmed that upon the abort of an offer, the entire collateral is expected to be returned to the maker. The problem is that in the current implementation, this does not work as expected, and the collateral corresponding to the usedPoints
is not returned. I will demonstrate this using the PreMarket.abortAskOffer
function.
First, here is a fragment from the sponsor's comment with a link to it.
https://discord.com/channels/1127263608246636635/1268902639555579954/1270804093463695502
I am also attaching the part of the PreMarket.abortAskOffer
function that shows how the corresponding refund amount is calculated. It is clearly seen that the entire collateral is reduced by the collateral corresponding to the recorded amount of usedPoints
(i.e., the points that were purchased by the taker).
Finally, I am attaching a proof of concept (POC) that demonstrates this in practice.
Loss of funds for the maker and wrong implementation of the contract based on the sponsor's comment
Manual review
Note, #148, #826, #1784 all stems from the fact that order statuses are not appropriately updated when a taker order is created against a maker offer. If the status is switched to `Ongoing/Filled` respectively, the virgin checks will fail appropriately and subsequently all this issues will be fixed. They could possibly be duplicates, so leaving open for appeal. This actually is closely related to issue #148. If a taker has created an Bid/Ask offer agains a original maker offer and the Status is adjusted to Ongoing/Filled accordingly, then this wrong refund won't occur, so could be duplicates, given only Virgin orders can be aborted (same applies for canceled orders, since only virgin orders are supposed to be able to be canceled). Since the status update is incorrect, then if a virgin order that is partially filled is aborted, the collateral is not returned correctly even though the order is not settled, causing a loss of funds to makers.
I disagree with the appeal above. the code is clearly not working as intended. Aborting offers means canceling all the deals and returning the collateral back to the offer maker. This is an obvious concept in the order-book model.
Concerning the sponsor's comment, he just answered questions in a public chat.
This issue is valid
"Aborting offers means canceling all the deals and returning the collateral back to the offer maker."
No, aborting offer means canceling all the deals and returning collateral corresponding to unused points. If some of the points are already used, the full amount won't be refunded to maker. Let me explain why this is designed in such a way:
If a maker creates a ASK offer and taker buys points from it.
When maker aborts the ASK offer, he should only get collateral corresponding to unused points. He shouldn't get full collateral because taker has already invested funds in his offer and that can also be claimed by the maker.
My question to submitter is,
"If maker should get full collateral back after aborting the offer, from which fund will taker claim his refund by calling "abortBidTaker"?"
The sponsors comment was totally wrong because the code is designed in such a way that maker gets partial collateral compared to unused points + taker's funds and taker can claim refund by calling "abortBidTaker"
@0xnevi, sorry if my first comment wasn't providing proper context.
The impact mentioned in the issue is:
"Loss of funds for the maker and wrong implementation of the contract based on the sponsor's comment"
Where is loss of funds for the maker?
| wrong implementation of the contract based on the sponsor's comment?
@0xnevi, we can't consider sponsor's comment and mention bugs based on that. You can check with equinous about that.
For hawk 8, you said:
No, aborting offer means canceling all the deals and returning collateral corresponding to unused points
This is incorrect, closeOffer
is used to close the offer and get refunded for the unused points. It seems you are misunderstanding some functions of the protocol
"If maker should get full collateral back after aborting the offer, from which fund will taker claim his refund by calling "abortBidTaker"?"
Yes, abortBidTaker
is called for takers to get refunded for aborted offers.
Where is loss of funds for the maker?
Read the function again and notice how the full collateral is not returned back to the offer maker when calling abortAsOffer
Again, the issue is valid as abortAskOffer
only refunds the offer maker with the unused portion of the amount (the margin)
Duplicate #887 described this issue better tho.
For Hawk 9,
"Read the function again and notice how the full collateral is not returned back to the offer maker when calling abortAsOffer
"
My response:
The maker will also get the fund that is sent by taker. So, he shouldn't get his full collateral back. check this function, https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L265-L273
https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L906-L949
maker is already getting funds corresponding to used points. His balance is updated which he can claim later. So, maker should get refund of unused points and not full collateral. Doing it otherwise will be loss to the protocols.
@nevi, if you add the following console.logs in the POC and rerun it:
following is printed:
That means maker deposited 12000000000000000. He can claim 3000000000000000 and gets refund of 9000000000000000.
The submitter says that there is loss to maker. Where is the loss. He is getting all his deposited collateral back
hawk 8 is correct, this issue and its duplicates are invalid and closely related to issue #826. The implementation is correct, abortAskOffer
should only be refunding unused points. The portion of 3000000000000000
will be present in the makers SalesRevenue
and is withdrawable when the taker creates a taker offer against the maker offer as seen here
Note, #148, #826, #1784 all stems from the fact that order statuses are not appropriately updated when a taker order is created against a maker offer. If the status is switched to `Ongoing/Filled` respectively, the virgin checks will fail appropriately and subsequently all this issues will be fixed. They could possibly be duplicates, so leaving open for appeal. This actually is closely related to issue #148. If a taker has created an Bid/Ask offer agains a original maker offer and the Status is adjusted to Ongoing/Filled accordingly, then this wrong refund won't occur, so could be duplicates, given only Virgin orders can be aborted (same applies for canceled orders, since only virgin orders are supposed to be able to be canceled). Since the status update is incorrect, then if a virgin order that is partially filled is aborted, the collateral is not returned correctly even though the order is not settled, causing a loss of funds to makers.
The contest is live. Earn rewards by submitting a finding.
Submissions are being reviewed by the community.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.