-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add FortaBridged contract #11
Conversation
Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
@@ -43,7 +47,7 @@ function prepare() { | |||
this.token .connect(this.accounts.admin).grantRole(this.roles.WHITELISTER, this.accounts.whitelister.address), | |||
this.otherToken.connect(this.accounts.admin).grantRole(this.roles.MINTER, this.accounts.minter.address ), | |||
this.otherToken.connect(this.accounts.admin).grantRole(this.roles.WHITELISTER, this.accounts.whitelister.address), | |||
].map(txPromise => txPromise.then(tx => tx.wait()))); | |||
].map(txPromise => txPromise.then(tx => tx.wait()).catch(() => {}))); |
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 is catch necessary?
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.
There is a like that is expected to revert in some case: When simulating L2, there is no MINTER
role setup on the Forta (which is a FortaBridged)
I thought this was the simplest way to discard this errors ... maybe not the safest but here we don't really care.
Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
Non-upgradeable BaseComponent is now dead code
@@ -74,7 +74,7 @@ describe('Dispatcher', function () { | |||
expect(await this.dispatch.scannerHash(this.SCANNER_ID)).to.not.be.deep.equal(hashBefore); | |||
}); | |||
|
|||
it('gas estimation', async function () { | |||
it.skip('gas estimation', async function () { |
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.
Can this be removed?
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 test is slow to run, and not relevant to security. We realized that gas restrictions for rpc view function calls is not an issue
I think this is good to merge! |
The FORT token needs to be bridged over to Polygon, where the system will operate and staking will happen.
We'll use the Polygon PoS Bridge. The token on Polygon will use the same code that was originally deployed to Ethereum Mainnet, extended with the functions required by the PoS Bridge (
deposit
andwithdraw
), and with the ability to mint removed.The roles of the token will be initially assigned to a Gnosis Safe deployed to Polygon with the same set of signers.