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

Smart contract #1

Merged
merged 17 commits into from
May 13, 2024
Merged

Smart contract #1

merged 17 commits into from
May 13, 2024

Conversation

AurelienFT
Copy link
Contributor

This PR contains the full code source for the DNS smart contract. This contract should match the NFT standard of Massa (inspired from ERC721), the RFC of the DNS SC here (with adjustments for NFT standards) and implement all features listed here

The two main files are

  • smart-contract/assembly/contracts/main.ts : Source code of the smart contract
  • smart-contract/assembly/__tests__/dns.spec.ts : Tests for each function of the smart contract

@damip Can you ensure the smart contract match all the needs please.

@gregLibert and @leoloco can you review the code please.

@AurelienFT AurelienFT requested review from damip, leoloco and gregLibert May 7, 2024 09:47
.github/workflows/ci-tests.yml Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
@AurelienFT AurelienFT requested a review from damip May 7, 2024 12:54
@AurelienFT AurelienFT requested a review from gregLibert May 7, 2024 12:54
Copy link

@leoloco leoloco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file structure is weird. Why is there a smart-contract folder that wraps everything ?
I don't understand why you use let for variables that do not change. Isn't it more time and space complex than const ?
Why don't you use expect + error message when deserializing user inputs ?

smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Outdated Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Show resolved Hide resolved
smart-contract/assembly/contracts/main.ts Show resolved Hide resolved
@AurelienFT
Copy link
Contributor Author

The file structure is weird. Why is there a smart-contract folder that wraps everything ? I don't understand why you use let for variables that do not change. Isn't it more time and space complex than const ? Why don't you use expect + error message when deserializing user inputs ?

This repository will also contains the website that will made for this project under another subdirectories.
For const and except, @gregLibert pointed it also and I have fixed it.

@AurelienFT AurelienFT requested a review from leoloco May 10, 2024 13:17
Copy link

@leoloco leoloco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AurelienFT AurelienFT merged commit 4283176 into main May 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants