block42-blockchain-company / liquid-icx

Secure the ICON network, earn staking rewards while staying liquid. 🧽💧
https://licx.finance
GNU General Public License v3.0
3 stars 1 forks source link

LICX Partial Audit #52

Open Spl3en opened 3 years ago

Spl3en commented 3 years ago

https://github.com/block42-blockchain-company/liquid-icx/blob/cd1987fb8d323625c25fdf79653dbad5baa909a6/score/liquid_icx/wallet.py#L9

https://github.com/block42-blockchain-company/liquid-icx/blob/cd1987fb8d323625c25fdf79653dbad5baa909a6/score/liquid_icx/wallet.py#L10

Severity : minor ℹ️

str(_address) may be more easy to read


https://github.com/block42-blockchain-company/liquid-icx/blob/cd1987fb8d323625c25fdf79653dbad5baa909a6/score/liquid_icx/wallet.py#L23

Severity : minor ℹ️

IIUC it doesn't make much sense to accept an unitialized node_id: int = None as a parameter. Keep this parameter mandatory


https://github.com/block42-blockchain-company/liquid-icx/blob/cd1987fb8d323625c25fdf79653dbad5baa909a6/score/liquid_icx/liquid_icx.py#L115-L117

Severity : minor ℹ️

Don't accept _address with a default None value if it isn't allowed


https://github.com/block42-blockchain-company/liquid-icx/blob/cd1987fb8d323625c25fdf79653dbad5baa909a6/score/liquid_icx/liquid_icx.py#L152

Severity : minor ℹ️

address should be Address type and then str'd to the sentinel, so the Address type checks are performed implicitely to the user input


https://github.com/block42-blockchain-company/liquid-icx/blob/cd1987fb8d323625c25fdf79653dbad5baa909a6/score/liquid_icx/liquid_icx.py#L152-L153

Severity : minor ℹ️

Does it make sense to return a list ? IIUC, this method should always return a single result. The select method from the LinkedListDB class tries to iterate through as much as possible in the list to look for all results matching the sentinel condition. Iterating manually (with a good old for loop) through the LinkedListDB and breaking when the first match is found may improve the SCORE performance


https://github.com/block42-blockchain-company/liquid-icx/blob/cd1987fb8d323625c25fdf79653dbad5baa909a6/score/liquid_icx/liquid_icx.py#L258

Severity : Critical ❌

self.icx.send may fail silentely which breaks your token logic : your internal wallet state will be updated, but the user may not receive his ICX correctly. You should check for self.icx.send return value, or use self.icx.transfer which raises an Exception if anything bad happens.

More information : https://www.icondev.io/docs/audit-checklist#section-unchecked-low-level-calls


https://github.com/block42-blockchain-company/liquid-icx/blob/cd1987fb8d323625c25fdf79653dbad5baa909a6/score/liquid_icx/liquid_icx.py#L423

Severity : minor ℹ️

A exists boolean method from the Wallet class would be easier to read and refactor than checking sender.node_id nonzero condition

mr-rooftop commented 3 years ago

Thank you very much for this partial audit update @Spl3en :100: We will consider everything and fix the affected lines.