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

[Multi ZNS] Replacing the meowToken with zToken #117

Open
wants to merge 14 commits into
base: rc/zns-meowchain
Choose a base branch
from

Conversation

MichaelKorchagin
Copy link
Collaborator

@MichaelKorchagin MichaelKorchagin commented Sep 12, 2024

MERGED TO: rc/multi-zns-main

@Whytecrowe Whytecrowe changed the base branch from development to rc/zns-meowchain September 12, 2024 20:34
package.json Outdated Show resolved Hide resolved
Copy link

openzeppelin-code bot commented Sep 12, 2024

Replacing the meowToken with zToken

Generated at commit: 76182b30799f969601a7cb96014159e297500929

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
1
1
0
2
17
21
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

Copy link

codecov bot commented Sep 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.80%. Comparing base (8513869) to head (76182b3).

Additional details and impacted files
@@                Coverage Diff                @@
##           rc/zns-meowchain     #117   +/-   ##
=================================================
  Coverage             99.80%   99.80%           
=================================================
  Files                    11       11           
  Lines                   525      525           
  Branches                117      117           
=================================================
  Hits                    524      524           
  Misses                    1        1           

hardhat.config.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@JamesEarle JamesEarle left a comment

Choose a reason for hiding this comment

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

Just the last few comments, otherwise looks good 👍

@@ -39,16 +39,16 @@ IZNSContracts
>) {
super(args);

if (this.config.mockMeowToken) {
this.contractName = znsNames.meowToken.contractMock;
if (this.config.mockZToken) {
Copy link
Collaborator

@Whytecrowe Whytecrowe Sep 24, 2024

Choose a reason for hiding this comment

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

now here it is a boolean already, so you can check it like this without problems. no change is needed here, just wanted to point out an example

}

// it will choose the governon as `admin` argument
// and deployAdmin as `minter` and first passed admin as `mintBeneficiary`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

no. all these addresses should be parts of the config. we need to be able to specify these addresses as needed.

initialAdminDelay : bigint;
initialSupplyBase : bigint;
inflationRates : Array<bigint>;
finalInflationRate : bigint;
Copy link
Collaborator

Choose a reason for hiding this comment

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

add all the constructor arguments for the contract here

@@ -628,6 +633,8 @@ describe("ZNSTreasury", () => {
const newHash = hashSubdomainName(newLabel);
const { expectedPrice, stakeFee } = getPriceObject(newLabel, DEFAULT_PRICE_CONFIG);

await zns.zToken.connect(deployer).approve(await zns.treasury.getAddress(), ethers.MaxInt256);
Copy link
Collaborator

Choose a reason for hiding this comment

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

again

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need an approval here, because the deployer is the participant of stakeForDomain function

Copy link
Collaborator

@Whytecrowe Whytecrowe left a comment

Choose a reason for hiding this comment

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

some more comments for you

@Whytecrowe Whytecrowe changed the title Replacing the meowToken with zToken [Multi ZNS] Replacing the meowToken with zToken Dec 17, 2024
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.

3 participants