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.

Sign up for GitHub

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

Rework upgradeable contracts initialization #75

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Rework upgradeable contracts initialization #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions contracts/upgradeable_contracts/ForeignOmnibridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ contract ForeignOmnibridge is BasicOmnibridge, GasLimitManager, InterestConnecto
uint256 _requestGasLimit,
address _owner,
address _tokenFactory
) external onlyRelevantSender returns (bool) {
) external onlyRelevantSender {
require(!isInitialized());

_setBridgeContract(_bridgeContract);
Expand All @@ -49,8 +49,6 @@ contract ForeignOmnibridge is BasicOmnibridge, GasLimitManager, InterestConnecto
_setTokenFactory(_tokenFactory);

setInitialize();

return isInitialized();
}

/**
Expand Down
4 changes: 1 addition & 3 deletions contracts/upgradeable_contracts/HomeOmnibridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ contract HomeOmnibridge is
address _tokenFactory,
address _feeManager,
address _forwardingRulesManager
) external onlyRelevantSender returns (bool) {
) external onlyRelevantSender {
require(!isInitialized());

_setBridgeContract(_bridgeContract);
Expand All @@ -59,8 +59,6 @@ contract HomeOmnibridge is
_setForwardingRulesManager(_forwardingRulesManager);

setInitialize();

return isInitialized();
}

/**
Expand Down
7 changes: 1 addition & 6 deletions contracts/upgradeable_contracts/Ownable.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import "../interfaces/IUpgradeabilityOwnerStorage.sol";
* @dev This contract has an owner address providing basic authorization control
*/
contract Ownable is EternalStorage {
bytes4 internal constant UPGRADEABILITY_OWNER = 0x6fde8202; // upgradeabilityOwner()

/**
* @dev Event to show ownership has been transferred
* @param previousOwner representing the address of the previous owner
Expand All @@ -36,11 +34,8 @@ contract Ownable is EternalStorage {
* @dev Throws if called through proxy by any account other than contract itself or an upgradeability owner.
*/
modifier onlyRelevantSender() {
(bool isProxy, bytes memory returnData) =
address(this).staticcall(abi.encodeWithSelector(UPGRADEABILITY_OWNER));
require(
!isProxy || // covers usage without calling through storage proxy
(returnData.length == 32 && msg.sender == abi.decode(returnData, (address))) || // covers usage through regular proxy calls
msg.sender == IUpgradeabilityOwnerStorage(address(this)).upgradeabilityOwner() || // covers usage through regular proxy calls
msg.sender == address(this) // covers calls through upgradeAndCall proxy method
);
_;
Expand Down
6 changes: 5 additions & 1 deletion test/omnibridge/WETHOmnibridgeRouter.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const ForeignOmnibridge = artifacts.require('ForeignOmnibridge')
const EternalStorageProxy = artifacts.require('EternalStorageProxy')
const AMBMock = artifacts.require('AMBMock')
const TokenFactory = artifacts.require('TokenFactory')
const WETHOmnibridgeRouter = artifacts.require('WETHOmnibridgeRouter')
Expand Down Expand Up @@ -27,7 +28,10 @@ contract('WETHOmnibridgeRouter', (accounts) => {

const tokenImage = await PermittableToken.new('TEST', 'TST', 18, 1337)
const tokenFactory = await TokenFactory.new(owner, tokenImage.address)
mediator = await ForeignOmnibridge.new(' on Testnet')
const proxy = await EternalStorageProxy.new()
const impl = await ForeignOmnibridge.new(' on Testnet')
await proxy.upgradeTo('1', impl.address)
mediator = await ForeignOmnibridge.at(proxy.address)
ambBridgeContract = await AMBMock.new()
await mediator.initialize(
ambBridgeContract.address,
Expand Down
20 changes: 4 additions & 16 deletions test/omnibridge/common.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,10 @@ function runTests(accounts, isHome) {
})

beforeEach(async () => {
contract = await Mediator.new(SUFFIX)
const proxy = await EternalStorageProxy.new()
const impl = await Mediator.new(SUFFIX)
await proxy.upgradeTo('1', impl.address)
contract = await Mediator.at(proxy.address)
ambBridgeContract = await AMBMock.new()
token = await ERC677BridgeToken.new('TEST', 'TST', 18)
currentDay = await contract.getCurrentDay()
Expand All @@ -162,9 +165,6 @@ function runTests(accounts, isHome) {

describe('claimTokens', () => {
beforeEach(async () => {
const storageProxy = await EternalStorageProxy.new()
await storageProxy.upgradeTo('1', contract.address).should.be.fulfilled
contract = await Mediator.at(storageProxy.address)
await initialize().should.be.fulfilled
})

Expand Down Expand Up @@ -850,15 +850,9 @@ function runTests(accounts, isHome) {

describe('fixMediatorBalance', () => {
beforeEach(async () => {
const storageProxy = await EternalStorageProxy.new()
await storageProxy.upgradeTo('1', contract.address).should.be.fulfilled
contract = await Mediator.at(storageProxy.address)

await token.mint(user, twoEthers, { from: owner }).should.be.fulfilled
await token.mint(contract.address, twoEthers, { from: owner }).should.be.fulfilled

await initialize().should.be.fulfilled

await token.transferAndCall(contract.address, oneEther, '0x', { from: user }).should.be.fulfilled

expect(await contract.mediatorBalance(token.address)).to.be.bignumber.equal(oneEther)
Expand Down Expand Up @@ -2273,9 +2267,6 @@ function runTests(accounts, isHome) {
})

beforeEach(async () => {
const storageProxy = await EternalStorageProxy.new()
await storageProxy.upgradeTo('1', contract.address).should.be.fulfilled
contract = await Mediator.at(storageProxy.address)
await initialize({
limits: [ether('100'), ether('99'), ether('0.01')],
executionLimits: [ether('100'), ether('99')],
Expand Down Expand Up @@ -2512,9 +2503,6 @@ function runTests(accounts, isHome) {
})

beforeEach(async () => {
const storageProxy = await EternalStorageProxy.new()
await storageProxy.upgradeTo('1', contract.address).should.be.fulfilled
contract = await Mediator.at(storageProxy.address)
await initialize({
limits: [ether('100'), ether('99'), ether('0.01')],
executionLimits: [ether('100'), ether('99')],
Expand Down