-
Notifications
You must be signed in to change notification settings - Fork 4
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
refactor: migrate repository to foundry-template #6
Conversation
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.
@3esmit I've left some inline comments.
There's probably more stuff we can adjust, but for starters, it introduces the foundry-template and also ensures tests are in parity.
| ------------ | -------- | -------- | ---------- | | ||
| | | | | | ||
| | | | | | ||
| | | | | |
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.
With these I think we need to put our heads together and figure out what the invariants are
} |
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.
Changes done in this file are just formatting to make the linter happy
} |
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 here, just formatting
function mint(address _destination, uint256 _amount) external onlyOwner { | ||
_mint(_destination, _amount); | ||
} | ||
} |
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've removed this one and added test/mocks/MockERC20.sol
contracts/legacy/Owned.sol
Outdated
} |
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.
Just formatting changes from forge fmt
contracts/legacy/SNTFaucet.sol
Outdated
} |
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 here, just formatting changes
contracts/legacy/SNTPlaceHolder.sol
Outdated
} |
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.
Formatting changes only
contracts/legacy/SafeMath.sol
Outdated
return a < b ? a : b; | ||
} | ||
} |
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.
Not sure about the entire legacy
folder here, but SafeMath
is most likely something we can drop altogether since solidity 0.8
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.
@3esmit Do you know if we need the legacy
folder at all?
It didn't seem to be used even prior to the changes I've made in this PR.
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.
We should be able to drop it
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 the legacy folder
assertEq(stakeManager.owner(), deployer); | ||
assertEq(stakeManager.totalSupply(), 0); | ||
} | ||
} |
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.
This essentially replaces StakeManager.js
constructor() ERC20("Mock SNT", "SNT") { | ||
_mint(msg.sender, 1_000_000_000_000_000_000); | ||
} | ||
} |
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.
This is the new mock that replaces Token.sol
5b11dc2
to
d44449e
Compare
This commit migrates the repo to our foundry template, which ensures we have consistent tooling across smart contract repositories that are maintained by Vac. This removes all hardhat related files and workflows and replaces them with more perfomant foundry workflows. It also sets up tests, CI and linting.
d44449e
to
d37ebd0
Compare
This commit migrates the repo to our foundry template, which ensures we have consistent tooling across smart contract repositories that are maintained by Vac.
This removes all hardhat related files and workflows and replaces them with more perfomant foundry workflows.
It also sets up tests, CI and linting.