rairprotocol / rair-solidity

public repo for RAIR solidity code
https://rairprotocol.org
186 stars 6 forks source link

Prepare diamonds codebase for Hats.finance audit #2

Open RAIReth opened 1 month ago

RAIReth commented 1 month ago

Help reduce the scope of the nSLOC for the hats.finance audit by commenting where we use existing pre-audited code, and where parts of the codebase are that are unique and need to be a focus of the audit.

Explanations of each contract and facet are documented in our Gitbook here.

https://docs.rairprotocol.org/rairprotocol/codebase/rairsolidity/all-contract-addresses

ben-kaufman commented 1 month ago

Hi, in your code I see a you are using the Diamond pattern, with multiple files which are standard to that pattern I assume were copy pasted from the standard library and do not require further auditing. Can you please confirm that and also if you wish to exclude any other parts of the code.

jmsm412 commented 1 month ago

@ben-kaufman Hi, yes the folder called diamondStandard has code from Nick Mudge's diamond-3, the other folder that's not necessary to check is maliciousContracts because that's just used in tests and are never deployed. We also use OpenZeppelin contracts but those are imported.

ben-kaufman commented 1 month ago

Thanks, an you also give a specific scope for the backend audit?

jmsm412 commented 1 month ago

@ben-kaufman I'm not sure what you mean by backend. The diamond-manager folder is just an UI to connect facets, it's not necessary to check.

RAIReth commented 1 month ago

@ben-kaufman I'm not sure what you mean by backend. The diamond-manager folder is just an UI to connect facets, it's not necessary to check.

We were chatting about auditing the diamond solidity first. Then the token credit system second (an offchain mongo entry that keeps track of a users token balance - vulnerability there is they could potentially use the system for free or another user is able to withdraw) if you checkout the 3 facets from the master list of diamond functions in that doc referenced above

Credit deposit query withdraw

RAIReth commented 1 month ago

We added a list of all the functions that can transfer fungibles and non fungibles in the system here.

https://docs.rairprotocol.org/rairprotocol/codebase/rairsolidity/transfer-functions

Hopefully that helps with the scope too. We re trying to ensure malicious funds cant transfer outside the users control. Eg deposits fungible tokens into credit system to spend and an unauthorized actor is able to withdraw to a different address. For the nft resales we offer both gas and gasless but have deprecated the gasless where an offchain db can sign for security reasons and require users to pay gas (or AA on their behalf) to post their resales onchain.

RAIReth commented 1 month ago

For design reasons (could be a lot of offchain activity to keep track of which makes paying gas for each discreet interaction expensive, or for points system you don't want tokens onchain until TGE) we have currently implemented the points/token credit system offchain but are open to suggestions there.

perezofir83 commented 1 month ago

Hey @jmsm412 @RAIReth, could you please direct us to the section where the backend is in your code and where the token credit system is implemented? There are a lot of files, and it will be easier for us to scope the backend part with some guidelines.

jmsm412 commented 1 month ago

@perezofir83 On the rair-dapp repository the backend code is in rair-node/bin/api/credits/credits.Service.js The result of the generateWithdrawRequest function is then used by the end-user in the frontend to call the withdraw function in the smart contract.

ben-kaufman commented 1 month ago

@jmsm412 so the file you mentioned is just about 50 lines of JS code, which is not really big enough for audit. In general, the whole code in the API folder seems quite simple, and I don't see much for an audit there.