code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

QA Report #125

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

QA Report

[L-01] Ownership transfer should be a two-step process

It is recommended that ownership transfer be completed in two steps, firstly by setting a pending admin and finally by accepting the change from the pending admin account.

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L428-L432 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Marketplace/MarketPlace.sol#L53-L56

[L-02] Floating pragma

Certain contracts contain a floating pragma. It is encouraged to deploy with the same specific compiler version for all contracts.

E.g. https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L2 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/Erc20.sol#L4

[N-01] Comment vs Code

The comment refers to "owner" but the param is named "holder".

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L94 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L120

robrobbins commented 2 years ago

1 is a wontfix 2 is fixed elsewhere N01 the zctoken is in flux and will go thru further scrutiny in its own process away from swivel