Security Testing Results for the Contract:

[liangping]

Security Testing Results for the Contract:

MakeSwap

(1) Uniqueness check: orderId=uuid+addrss, error reported in case of duplicates, validation correct!
(2) Parameter compliance check: Correct!
(3) Fund access check: Correct!

MakeSwapWithVesting

(1) Added compliance check for Vesting parameters on top of MakeSwap!
Issue: Currently, in Vesting parameters, no step's percentage is allowed to be equal to 0! (Is this design reasonable?)

TakeSwap

(1) Order status check: Correct!
(2) Parameter compliance check: Correct!
(3) Order expiration check: Correct!
(4) Fund access check: Correct!

CancelSwap

(1) Order status check: Correct!
(2) Parameter compliance check: Correct!
(3) Fund access check: Correct!
Issue: After canceling the order, the contract deletes the order, and subsequently related bids cannot correctly be claimed! (Severe)

PlaceBid

(1) Order status check: Correct!
(2) Uniqueness check: Each user can only placeBid once; error reported in case of duplicates, validation correct!
(3) Order expiration check: Correct!
(4) Parameter compliance check: Correct!
(5) Fund access check: Correct!
Issue: Not checking if the order allows bidding, i.e., if acceptBid is true? (Incorrect logic, not severely harmful)

UpdateBid

(1) Order status check: Correct!
(2) Bid status check: Correct!
(3) Bid expiration check: Correct!
(4) Parameter compliance check: Correct!
(5) Fund access check: Correct!

AcceptBid

(1) Order status check: Correct!
(2) Check acceptBid=true: Correct!
(3) Bid status check: Correct!
(4) Bid expiration check: Correct!
(5) Fund access check: Correct!

CancelBid

(1) Order existence check: Correct!
(2) Bid status check: Correct!
(3) Parameter compliance check: Correct!
(4) Fund access check: Correct!

StartVesting

(1) Added administrator mechanism, only specified addresses are allowed to call this interface.
(2) Parameter compliance check: Correct!
(3) Uniqueness check, beneficiary+orderId unique to avoid data overwrite attacks, correct!
(4) Fund access check: Correct!

Release

(1) Ensures the uniqueness of each Vesting detail execution, no duplicate claims, total asset payment correct!
Issue: Calculation problem in claiming Vesting, can be claimed prematurely! (Severe)

Issue List:

• 1. PlaceBid does not check if the order allows bidding, i.e., if acceptBid is true? (Incorrect logic, not severely harmful)
• 2. CancelSwap deletes the order, and subsequently related bids cannot correctly be claimed! (Severe)
• 3. When submitting a Vesting order, currently, Vesting parameters don't allow any step's percentage to be equal to 0! (Is this design reasonable?)
• 4. In Release Vesting, the calculation of Vesting time is problematic; it can be claimed prematurely! (Severe)

[liangping]

issue 3: we can don't save T0 on chain. if both duration and percentage are 0. instead, we need improve this issue on front-end. @thmadong @alisaweb3

[liangping]

Issue 4: fix patch

 uint256 releaseTime = schedule.start;
// for (uint256 i = schedule.lastReleasedStep; i < releases.length; i++) {
for (uint256 i = 0; i < releases.length; i++) {
    releaseTime += releases[i].durationInHours * 1 hours;
    if (i < schedule.lastReleasedStep) continue // continue to next
    if (block.timestamp < releaseTime) {
        break;
    }
    uint256 releaseAmount = (schedule.totalAmount *
        releases[i].percentage) / 10000;
    amountForRelease += releaseAmount;
    schedule.lastReleasedStep = i + 1;
}

[soring323]

When submitting a Vesting order, currently, Vesting parameters don't allow any step's percentage to be equal to 0! (Is this design reasonable?)

is there need to add 0 percent release?
why do we need this?

[soring323]

4. In Release Vesting, the calculation of Vesting time is problematic; it can be claimed prematurely! (Severe)

of course.
but there is no release always.
because we are saving laststep.

[soring323]

so can't release anyway.

[liangping]

There is no problem on release step.
The problem is on the calculation of release time. you can see the right time of each term in following table.

Term	duration	release time
T0	D0	start_time + D0
T1	D1	start_time + D0 + D1
T2	D2	start_time + D0 + D1 + D2
T3	D3	start_time + D0 + D1 + D2 + D3
T4	D4	start_time + D0 + D1 + D2 + D3+ D4

But the duration of claimed releases have been omitted in your previous implementation,

Time of T3 become start time + D3

[soring323]

ok