Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare diamonds codebase for Hats.finance audit #2

Open
RAIReth opened this issue Sep 12, 2024 · 10 comments
Open

Prepare diamonds codebase for Hats.finance audit #2

RAIReth opened this issue Sep 12, 2024 · 10 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@RAIReth
Copy link

RAIReth commented Sep 12, 2024

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

@RAIReth RAIReth added the documentation Improvements or additions to documentation label Sep 12, 2024
@ben-kaufman
Copy link

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
Copy link
Collaborator

jmsm412 commented Sep 13, 2024

@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
Copy link

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

@jmsm412
Copy link
Collaborator

jmsm412 commented Sep 16, 2024

@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
Copy link
Author

RAIReth commented Sep 17, 2024

@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
Copy link
Author

RAIReth commented Sep 17, 2024

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
Copy link
Author

RAIReth commented Sep 17, 2024

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
Copy link

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
Copy link
Collaborator

jmsm412 commented Sep 19, 2024

@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
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Todo
Development

No branches or pull requests

4 participants