-
Notifications
You must be signed in to change notification settings - Fork 166
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
feat: add nft and token linker contracts #144
base: main
Are you sure you want to change the base?
Conversation
Hey, thanks for the PR 🙌🏻 Can you add a script to |
No problem @npty! And yes definitely, I will add those scripts once we wrap up with audits and releasing governance. |
address gateway_, | ||
address gasReceiver_, | ||
address owner_ | ||
) ERC721('Axelar NFT Linker', 'ANL') AxelarExecutable(gateway_) Upgradable(owner_) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
) ERC721('Axelar NFT Linker', 'ANL') AxelarExecutable(gateway_) Upgradable(owner_) { | |
) ERC721('Axelar NFT Linker', 'ANL') AxelarExecutable(gateway_) Upgradable(msg.sender) { |
Just use msg.sender for these example contracts for simplicity and avoids breaking compatibility
|
||
constructor(address implementationAddress, address owner, bytes memory setupParams) Proxy(implementationAddress, owner, setupParams) {} | ||
|
||
// slither-disable-next-line dead-code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// slither-disable-next-line dead-code |
add the same slither/solhint config to this repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added solhint/slither configs to ignore these warnings
hardhat.config.js
Outdated
@@ -1,3 +1,4 @@ | |||
require("@nomiclabs/hardhat-ethers"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not import the toolbox here?
package.json
Outdated
@@ -31,8 +31,10 @@ | |||
"uuid": "^8.3.2" | |||
}, | |||
"devDependencies": { | |||
"@nomiclabs/hardhat-ethers": "^2.2.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not use the toolbox and remove this and gas reporter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this, I tried to install toolbox and remove gas reporter but ran into a lot of dependency conflicts between the toolbox ethers and ethers in the dependencies. It seems like we are only using the gas reporter package from the toolbox so maybe importing the entire toolbox at the point is unnecessary?
package.json
Outdated
@@ -31,8 +31,10 @@ | |||
"uuid": "^8.3.2" | |||
}, | |||
"devDependencies": { | |||
"@nomiclabs/hardhat-ethers": "^2.2.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now that tests were removed this shouldn't be needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
hardhat.config.js
Outdated
@@ -1,3 +1,4 @@ | |||
require('@nomiclabs/hardhat-ethers'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
PR to add NFT and Token linker contracts and tests to the examples repo.