Skip to content

Conversation

@MartinGbz
Copy link
Contributor

This PR update aave-helpers lib (in order to get the new assets from aave-address-book).

Due to these last commits from bgd-labs/solidity-utils:

I needed to use directely openzeppelin-contracts, instead of former oz-common folder contracts.

@MartinGbz
Copy link
Contributor Author

Just a remark, I saw that 2 openzeppelin contracts were already imported in RiskOracle.sol:

import '@openzeppelin/contracts/access/Ownable.sol';
import '@openzeppelin/contracts/utils/Strings.sol';

Are these imports correct?

@brotherlymite
Copy link
Collaborator

@MartinGbz can you also bump solc to '0.8.22' for both zksync and general on foundry.toml otherwise it would not compile

@brotherlymite
Copy link
Collaborator

Just a remark, I saw that 2 openzeppelin contracts were already imported in RiskOracle.sol:

import '@openzeppelin/contracts/access/Ownable.sol';
import '@openzeppelin/contracts/utils/Strings.sol';

Are these imports correct?

fine to ignore those as it is only used on tests, ideally wanted to add RiskOracle.sol from their repo as submodule but didn't do that time as it required min 0.8.25 solc version on their contract

@MartinGbz
Copy link
Contributor Author

@MartinGbz can you also bump solc to '0.8.22' for both zksync and general on foundry.toml otherwise it would not compile

On my side it compiled even with 0.8.20, but i bumped it to 0.8.22 👍

Co-authored-by: Harsh Pandey <[email protected]>
@brotherlymite
Copy link
Collaborator

@MartinGbz on the tests, can you replace vm.expectRevert(bytes('Ownable: caller is not the owner')); with something like vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, address(1))); so the tests pass

@brotherlymite brotherlymite merged commit 9d11b05 into aave-dao:main Feb 4, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants