Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 4 additions & 28 deletions contracts/facets/IexecEscrowTokenFacet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,7 @@ contract IexecEscrowTokenFacet is IexecEscrowToken, IexecTokenSpender, IexecERC2
) external override returns (bool) {
PocoStorageLib.PocoStorage storage $ = PocoStorageLib.getPocoStorage();
_transfer(sender, recipient, amount);
// TEMPORARY MIGRATION FIX: Check allowance to prevent underflow and revert without reason for backward compatibility
// TODO: Remove this in the next major version
uint256 currentAllowance = $.m_allowances[sender][_msgSender()];
if (currentAllowance < amount) {
revert();
}
_approve(sender, _msgSender(), currentAllowance - amount);
_approve(sender, _msgSender(), $.m_allowances[sender][_msgSender()] - amount);
return true;
}

Expand All @@ -262,27 +256,15 @@ contract IexecEscrowTokenFacet is IexecEscrowToken, IexecTokenSpender, IexecERC2
uint256 subtractedValue
) external override returns (bool) {
PocoStorageLib.PocoStorage storage $ = PocoStorageLib.getPocoStorage();
// TEMPORARY MIGRATION FIX: Check allowance to prevent underflow and revert without reason for backward compatibility
// TODO: Remove this in the next major version
uint256 currentAllowance = $.m_allowances[_msgSender()][spender];
if (currentAllowance < subtractedValue) {
revert();
}
_approve(_msgSender(), spender, currentAllowance - subtractedValue);
_approve(_msgSender(), spender, $.m_allowances[_msgSender()][spender] - subtractedValue);
return true;
}

function _transferUnchecked(address sender, address recipient, uint256 amount) internal {
require(sender != address(0), "ERC20: transfer from the zero address");
require(recipient != address(0), "ERC20: transfer to the zero address");
PocoStorageLib.PocoStorage storage $ = PocoStorageLib.getPocoStorage();
uint256 senderBalance = $.m_balances[sender];
// TEMPORARY MIGRATION FIX: Check balance to prevent underflow and revert without reason for backward compatibility
// TODO: Remove this in the next major version
if (senderBalance < amount) {
revert();
}
$.m_balances[sender] = senderBalance - amount;
$.m_balances[sender] = $.m_balances[sender] - amount;
$.m_balances[recipient] = $.m_balances[recipient] + amount;
emit Transfer(sender, recipient, amount);
}
Expand All @@ -302,14 +284,8 @@ contract IexecEscrowTokenFacet is IexecEscrowToken, IexecTokenSpender, IexecERC2
function _burn(address account, uint256 amount) internal {
require(account != address(0), "ERC20: burn from the zero address");
PocoStorageLib.PocoStorage storage $ = PocoStorageLib.getPocoStorage();
uint256 accountBalance = $.m_balances[account];
// TEMPORARY MIGRATION FIX: Check balance to prevent underflow and revert without reason for backward compatibility
// TODO: Remove this in the next major version
if (accountBalance < amount) {
revert();
}
$.m_totalSupply = $.m_totalSupply - amount;
$.m_balances[account] = accountBalance - amount;
$.m_balances[account] = $.m_balances[account] - amount;
emit Transfer(account, address(0), amount);
}

Expand Down
16 changes: 1 addition & 15 deletions contracts/registries/Registry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,6 @@ abstract contract Registry is IRegistry, ERC721Enumerable, Ownable {
proxyCodeHash = keccak256(proxyCode);
}

// TEMPORARY MIGRATION FIX: Override _checkOwner to catch custom errors and throw string errors for backward compatibility
// TODO: Remove this override in the next major version
function _checkOwner() internal view override {
if (owner() != _msgSender()) {
revert("Ownable: caller is not the owner");
}
}

function initialize(address _previous) external onlyOwner {
require(!initialized);
initialized = true;
Expand Down Expand Up @@ -70,14 +62,8 @@ abstract contract Registry is IRegistry, ERC721Enumerable, Ownable {

/* Factory */
function _mintCreate(address _owner, bytes memory _args) internal returns (address) {
// TEMPORARY MIGRATION FIX: Check if contract already exists to revert without custom error for backward compatibility
// TODO: Remove this in the next major version
address entry = _mintPredict(_owner, _args);
if (entry.code.length > 0) {
revert("Create2: Failed on deploy");
}
// Create entry (proxy)
entry = Create2.deploy(0, keccak256(abi.encodePacked(_args, _owner)), proxyCode);
address entry = Create2.deploy(0, keccak256(abi.encodePacked(_args, _owner)), proxyCode);
InitializableUpgradeabilityProxy(payable(entry)).initialize(master, _args);
// Mint corresponding token
_mint(_owner, uint256(uint160(entry)));
Expand Down
8 changes: 4 additions & 4 deletions test/byContract/IexecERC20/IexecERC20.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('ERC20', async () => {
it('Should not transfer when sender balance is too low', async () => {
await expect(
iexecPocoAsHolder.transfer(recipient.address, value + 1n),
).to.be.revertedWithoutReason();
).to.be.revertedWithPanic(0x11);
});
});

Expand Down Expand Up @@ -201,13 +201,13 @@ describe('ERC20', async () => {
it('Should not transferFrom when owner balance is too low', async () => {
await expect(
iexecPocoAsSpender.transferFrom(holder.address, spender.address, value + 1n),
).to.be.revertedWithoutReason();
).to.be.revertedWithPanic(0x11);
});
it('Should not transferFrom when spender allowance is too low', async () => {
await iexecPocoAsHolder.approve(spender.address, value - 1n).then((tx) => tx.wait());
await expect(
iexecPocoAsSpender.transferFrom(holder.address, spender.address, value),
).to.be.revertedWithoutReason();
).to.be.revertedWithPanic(0x11);
});
});

Expand Down Expand Up @@ -260,7 +260,7 @@ describe('ERC20', async () => {
it('Should not decrease allowance of a value greater than old allowance', async () => {
await expect(
iexecPocoAsHolder.decreaseAllowance(spender.address, 1),
).to.be.revertedWithoutReason();
).to.be.revertedWithPanic(0x11);
});
it('Should not decrease allowance from the zero address', async () => {
await expect(
Expand Down
8 changes: 4 additions & 4 deletions test/byContract/IexecEscrow/IexecEscrowToken.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,13 @@ describe('IexecEscrowToken', () => {
.withArgs(proxyAddress, accountA.address, 0);
});
it('Should not withdraw tokens with empty balance', async () => {
await expect(iexecPocoAsAccountA.withdraw(amount)).to.be.revertedWithoutReason();
await expect(iexecPocoAsAccountA.withdraw(amount)).to.be.revertedWithPanic(0x11);
});
it('Should not withdraw tokens with insufficient balance', async () => {
await rlcInstanceAsAccountA.approve(proxyAddress, amount).then((tx) => tx.wait());
await iexecPocoAsAccountA.deposit(amount).then((tx) => tx.wait());

await expect(iexecPocoAsAccountA.withdraw(amount + 1n)).to.be.revertedWithoutReason();
await expect(iexecPocoAsAccountA.withdraw(amount + 1n)).to.be.revertedWithPanic(0x11);
});
});

Expand Down Expand Up @@ -366,15 +366,15 @@ describe('IexecEscrowToken', () => {
it('Should not withdraw to another address with empty balance', async () => {
await expect(
iexecPocoAsAccountA.withdrawTo(amount, accountB.address),
).to.be.revertedWithoutReason();
).to.be.revertedWithPanic(0x11);
});
it('Should not withdraw to another address with insufficient balance', async () => {
await rlcInstanceAsAccountA.approve(proxyAddress, amount).then((tx) => tx.wait());
await iexecPocoAsAccountA.deposit(amount).then((tx) => tx.wait());

await expect(
iexecPocoAsAccountA.withdrawTo(amount + 1n, accountB.address),
).to.be.revertedWithoutReason();
).to.be.revertedWithPanic(0x11);
});
});

Expand Down
31 changes: 17 additions & 14 deletions test/byContract/registries/registries.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,17 @@ describe('Registries', () => {
});

it('Should not initialize when user is not the owner', async () => {
await expect(appRegistry.initialize(ZeroAddress)).to.be.revertedWith(
'Ownable: caller is not the owner',
await expect(appRegistry.initialize(ZeroAddress)).to.be.revertedWithCustomError(
appRegistry,
'OwnableUnauthorizedAccount',
);
await expect(datasetRegistry.initialize(ZeroAddress)).to.be.revertedWith(
'Ownable: caller is not the owner',
await expect(datasetRegistry.initialize(ZeroAddress)).to.be.revertedWithCustomError(
datasetRegistry,
'OwnableUnauthorizedAccount',
);
await expect(workerpoolRegistry.initialize(ZeroAddress)).to.be.revertedWith(
'Ownable: caller is not the owner',
await expect(workerpoolRegistry.initialize(ZeroAddress)).to.be.revertedWithCustomError(
workerpoolRegistry,
'OwnableUnauthorizedAccount',
);
});

Expand All @@ -131,15 +134,15 @@ describe('Registries', () => {
});

it('Should not set base URI when user is not the owner', async () => {
await expect(appRegistry.setBaseURI(`https://new.url.iex.ec/app/`)).to.be.revertedWith(
'Ownable: caller is not the owner',
);
await expect(
appRegistry.setBaseURI(`https://new.url.iex.ec/app/`),
).to.be.revertedWithCustomError(appRegistry, 'OwnableUnauthorizedAccount');
await expect(
datasetRegistry.setBaseURI(`https://new.url.iex.ec/dataset/`),
).to.be.revertedWith('Ownable: caller is not the owner');
).to.be.revertedWithCustomError(datasetRegistry, 'OwnableUnauthorizedAccount');
await expect(
workerpoolRegistry.setBaseURI(`https://new.url.iex.ec/workerpool/`),
).to.be.revertedWith('Ownable: caller is not the owner');
).to.be.revertedWithCustomError(workerpoolRegistry, 'OwnableUnauthorizedAccount');
});
});

Expand Down Expand Up @@ -280,7 +283,7 @@ describe('Registries', () => {

await expect(
appRegistry.createApp(appProvider.address, ...createAppArgs),
).to.be.revertedWith('Create2: Failed on deploy');
).to.be.revertedWithCustomError(appRegistry, 'Create2FailedDeployment');
});

it('Should check that a new app is well registered on new app registry', async () => {
Expand Down Expand Up @@ -385,7 +388,7 @@ describe('Registries', () => {

await expect(
datasetRegistry.createDataset(datasetProvider.address, ...createDatasetArgs),
).to.be.revertedWith('Create2: Failed on deploy');
).to.be.revertedWithCustomError(datasetRegistry, 'Create2FailedDeployment');
});
});

Expand Down Expand Up @@ -469,7 +472,7 @@ describe('Registries', () => {

await expect(
workerpoolRegistry.createWorkerpool(scheduler.address, ...createWorkerpoolArgs),
).to.be.revertedWith('Create2: Failed on deploy');
).to.be.revertedWithCustomError(workerpoolRegistry, 'Create2FailedDeployment');
});
});
});
Loading