fix: Remove external hacky getProxyImplementation function

contracts/interfaces/IPoolAddressesProvider.sol

@@ -105,13 +105,6 @@ interface IPoolAddressesProvider {
    */
   function getAddress(bytes32 id) external view returns (address);
 
-  /**
-   * @notice Returns the implementation address of the proxy contract.
-   * @param id The id
-   * @return The implementation address of the proxy, or ZERO address if it is not a proxy
-   */
-  function getProxyImplementation(bytes32 id) external view returns (address);
-
   /**
    * @notice General function to update the implementation of a proxy registered with
    * certain `id`. If there is no proxy registered, it will instantiate one and

contracts/protocol/configuration/PoolAddressesProvider.sol

@@ -57,19 +57,6 @@ contract PoolAddressesProvider is Ownable, IPoolAddressesProvider {
     emit AddressSet(id, newAddress, false);
   }
 
-  /// @inheritdoc IPoolAddressesProvider
-  function getProxyImplementation(bytes32 id) public view override returns (address) {
-    address proxyAddress = _addresses[id];
-    if (proxyAddress == address(0)) {
-      return address(0);
-    } else {
-      (bool success, bytes memory result) = proxyAddress.staticcall(
-        abi.encodeWithSignature('implementation()')
-      );
-      return success && result.length == 32 ? result.toAddress() : address(0);
-    }
-  }
-
   /// @inheritdoc IPoolAddressesProvider
   function setAddressAsProxy(bytes32 id, address implementationAddress)
     external
@@ -87,7 +74,7 @@ contract PoolAddressesProvider is Ownable, IPoolAddressesProvider {
 
   /// @inheritdoc IPoolAddressesProvider
   function setPoolImpl(address newPoolImpl) external override onlyOwner {
-    address oldPoolImpl = getProxyImplementation(POOL);
+    address oldPoolImpl = _getProxyImplementation(POOL);
     _updateImpl(POOL, newPoolImpl);
     emit PoolUpdated(oldPoolImpl, newPoolImpl);
   }
@@ -99,7 +86,7 @@ contract PoolAddressesProvider is Ownable, IPoolAddressesProvider {
 
   /// @inheritdoc IPoolAddressesProvider
   function setPoolConfiguratorImpl(address newPoolConfiguratorImpl) external override onlyOwner {
-    address oldPoolConfiguratorImpl = getProxyImplementation(POOL_CONFIGURATOR);
+    address oldPoolConfiguratorImpl = _getProxyImplementation(POOL_CONFIGURATOR);
     _updateImpl(POOL_CONFIGURATOR, newPoolConfiguratorImpl);
     emit PoolConfiguratorUpdated(oldPoolConfiguratorImpl, newPoolConfiguratorImpl);
   }
@@ -205,4 +192,21 @@ contract PoolAddressesProvider is Ownable, IPoolAddressesProvider {
     _marketId = newMarketId;
     emit MarketIdSet(oldMarketId, newMarketId);
   }
+
+  /**
+   * @notice Returns the the implementation contract of the proxy contract by its identifier.
+   * @dev It returns ZERO if there is no registered address with the given id
+   * @dev It reverts if the registered address with the given id is not `InitializableImmutableAdminUpgradeabilityProxy`
+   * @param id The id
+   * @return The address of the implementation contract
+   */
+  function _getProxyImplementation(bytes32 id) internal returns (address) {
+    address proxyAddress = _addresses[id];
+    if (proxyAddress == address(0)) {
+      return address(0);
+    } else {
+      address payable payableProxyAddress = payable(proxyAddress);
+      return InitializableImmutableAdminUpgradeabilityProxy(payableProxyAddress).implementation();
+    }
+  }
 }

test-suites/aave-protocol-data-provider.spec.ts

@@ -2,12 +2,14 @@ import hre from 'hardhat';
 import { expect } from 'chai';
 import { utils } from 'ethers';
 import { makeSuite, TestEnv } from './helpers/make-suite';
-import { getMockPool } from '@aave/deploy-v3';
+import { getMockPool, ZERO_ADDRESS } from '@aave/deploy-v3';
+import { InitializableImmutableAdminUpgradeabilityProxy } from '../types';
+import { impersonateAccountsHardhat } from '../helpers/misc-utils';
+import { topUpNonPayableWithEther } from './helpers/utils/funds';
 
 makeSuite('AaveProtocolDataProvider: Edge cases', (testEnv: TestEnv) => {
   const MKR_ADDRESS = '0x9f8F72aA9304c8B593d555F12eF6589cC3A579A2';
   const ETH_ADDRESS = '0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE';
-  const POOL_ID = utils.formatBytes32String('POOL');
 
   it('getAllReservesTokens() with MKR and ETH as symbols', async () => {
     const { addressesProvider, poolAdmin, helpersContract } = testEnv;
@@ -16,7 +18,18 @@ makeSuite('AaveProtocolDataProvider: Edge cases', (testEnv: TestEnv) => {
     // Deploy a mock Pool
     const mockPool = await hre.deployments.deploy('MockPool', { from: deployer });
 
-    const oldPoolImpl = await addressesProvider.getProxyImplementation(POOL_ID);
+    // Impersonate PoolAddressesProvider
+    await impersonateAccountsHardhat([addressesProvider.address]);
+    const addressesProviderSigner = await hre.ethers.getSigner(addressesProvider.address);
+
+    const poolProxyAddress = await addressesProvider.getPool();
+    const poolProxy = (await hre.ethers.getContractAt(
+      'InitializableImmutableAdminUpgradeabilityProxy',
+      poolProxyAddress,
+      addressesProviderSigner
+    )) as InitializableImmutableAdminUpgradeabilityProxy;
+
+    const oldPoolImpl = await poolProxy.callStatic.implementation();
 
     // Update the addressesProvider with a mock pool
     expect(await addressesProvider.connect(poolAdmin.signer).setPoolImpl(mockPool.address))

test-suites/pool-addresses-provider.spec.ts

@@ -1,12 +1,28 @@
 import hre from 'hardhat';
 import { expect } from 'chai';
 import { utils } from 'ethers';
-import { createRandomAddress } from '../helpers/misc-utils';
+import { createRandomAddress, impersonateAccountsHardhat } from '../helpers/misc-utils';
 import { ProtocolErrors } from '../helpers/types';
 import { ZERO_ADDRESS } from '../helpers/constants';
 import { makeSuite, TestEnv } from './helpers/make-suite';
 import { deployPool, deployMockPool } from '@aave/deploy-v3/dist/helpers/contract-deployments';
 import { evmSnapshot, evmRevert } from '@aave/deploy-v3';
+import { InitializableImmutableAdminUpgradeabilityProxy } from '../types';
+
+const getProxyImplementation = async (addressesProviderAddress: string, proxyAddress: string) => {
+  // Impersonate PoolAddressesProvider
+  await impersonateAccountsHardhat([addressesProviderAddress]);
+  const addressesProviderSigner = await hre.ethers.getSigner(addressesProviderAddress);
+
+  const proxy = (await hre.ethers.getContractAt(
+    'InitializableImmutableAdminUpgradeabilityProxy',
+    proxyAddress,
+    addressesProviderSigner
+  )) as InitializableImmutableAdminUpgradeabilityProxy;
+
+  const implementationAddress = await proxy.callStatic.implementation();
+  return implementationAddress;
+};
 
 makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
   const { OWNABLE_ONLY_OWNER } = ProtocolErrors;
@@ -60,7 +76,8 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
       .withArgs(proxiedAddressId, mockPool.address, true)
       .to.emit(addressesProvider, 'ProxyCreated');
 
-    const implAddress = await addressesProvider.getProxyImplementation(proxiedAddressId);
+    const proxyAddress = await addressesProvider.getAddress(proxiedAddressId);
+    const implAddress = await getProxyImplementation(addressesProvider.address, proxyAddress);
     expect(implAddress).to.be.eq(mockPool.address);
   });
 
@@ -83,8 +100,8 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
       mockNonProxiedAddress.toLowerCase()
     );
 
-    const impAddress = await addressesProvider.getProxyImplementation(nonProxiedAddressId);
-    expect(impAddress).to.be.eq(ZERO_ADDRESS);
+    const proxyAddress = await addressesProvider.getAddress(nonProxiedAddressId);
+    await expect(getProxyImplementation(addressesProvider.address, proxyAddress)).to.be.reverted;
   });
 
   it('Owner adds a new address with no proxy and turns it into a proxy', async () => {
@@ -108,9 +125,8 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
 
     let registeredAddress = await addressesProvider.getAddress(convertibleAddressId);
     expect(registeredAddress).to.be.eq(mockConvertibleAddress);
-    expect(await addressesProvider.getProxyImplementation(convertibleAddressId)).to.be.eq(
-      ZERO_ADDRESS
-    );
+    await expect(getProxyImplementation(addressesProvider.address, registeredAddress)).to.be
+      .reverted;
 
     // Unregister address as non proxy
     expect(
@@ -131,9 +147,9 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
       .withArgs(convertibleAddressId, mockConvertibleAddress, true)
       .to.emit(addressesProvider, 'ProxyCreated');
 
-    expect(await addressesProvider.getProxyImplementation(convertibleAddressId)).to.be.eq(
-      mockConvertibleAddress
-    );
+    const proxyAddress = await addressesProvider.getAddress(convertibleAddressId);
+    const implAddress = await getProxyImplementation(addressesProvider.address, proxyAddress);
+    expect(implAddress).to.be.eq(mockConvertibleAddress);
   });
 
   it('Unregister a proxy address', async () => {
@@ -144,10 +160,8 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
     const convertibleAddressId = utils.formatBytes32String('CONVERTIBLE_ADDRESS');
 
     const proxyAddress = await addressesProvider.getAddress(convertibleAddressId);
-    const implementationAddress = await addressesProvider.getProxyImplementation(
-      convertibleAddressId
-    );
-    expect(implementationAddress).to.be.not.eq(ZERO_ADDRESS);
+    const implAddress = await getProxyImplementation(addressesProvider.address, proxyAddress);
+    expect(implAddress).to.be.not.eq(ZERO_ADDRESS);
 
     expect(
       await addressesProvider
@@ -160,11 +174,8 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
     const proxyAddressAfter = await addressesProvider.getAddress(convertibleAddressId);
     expect(proxyAddressAfter).to.be.eq(ZERO_ADDRESS);
     expect(proxyAddressAfter).to.be.not.eq(proxyAddress);
-    const implementationAddressAfter = await addressesProvider.getProxyImplementation(
-      convertibleAddressId
-    );
-    expect(implementationAddressAfter).to.be.eq(ZERO_ADDRESS);
-    expect(implementationAddressAfter).to.be.not.eq(implementationAddress);
+    await expect(getProxyImplementation(addressesProvider.address, proxyAddressAfter)).to.be
+      .reverted;
   });
 
   it('Owner adds a new address with proxy and turns it into a no proxy', async () => {
@@ -188,10 +199,8 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
       .to.emit(addressesProvider, 'ProxyCreated');
 
     const proxyAddress = await addressesProvider.getAddress(convertibleAddressId);
-    const implementationAddress = await addressesProvider.getProxyImplementation(
-      convertibleAddressId
-    );
-    expect(implementationAddress).to.be.eq(mockConvertibleAddress);
+    const implAddress = await getProxyImplementation(addressesProvider.address, proxyAddress);
+    expect(implAddress).to.be.eq(mockConvertibleAddress);
 
     // Unregister address as proxy
     expect(
@@ -214,9 +223,8 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
     const registeredAddressAfter = await addressesProvider.getAddress(convertibleAddressId);
     expect(registeredAddressAfter).to.be.not.eq(proxyAddress);
     expect(registeredAddressAfter).to.be.eq(mockConvertibleAddress);
-    expect(await addressesProvider.getProxyImplementation(convertibleAddressId)).to.be.eq(
-      ZERO_ADDRESS
-    );
+    await expect(getProxyImplementation(addressesProvider.address, registeredAddressAfter)).to.be
+      .reverted;
   });
 
   it('Unregister a no proxy address', async () => {
@@ -227,10 +235,8 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
     const convertibleAddressId = utils.formatBytes32String('CONVERTIBLE_ADDRESS2');
 
     const registeredAddress = await addressesProvider.getAddress(convertibleAddressId);
-    const implementationAddress = await addressesProvider.getProxyImplementation(
-      convertibleAddressId
-    );
-    expect(implementationAddress).to.be.eq(ZERO_ADDRESS);
+    await expect(getProxyImplementation(addressesProvider.address, registeredAddress)).to.be
+      .reverted;
 
     expect(
       await addressesProvider
@@ -243,10 +249,8 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
     const registeredAddressAfter = await addressesProvider.getAddress(convertibleAddressId);
     expect(registeredAddressAfter).to.be.eq(ZERO_ADDRESS);
     expect(registeredAddressAfter).to.be.not.eq(registeredAddress);
-    const implementationAddressAfter = await addressesProvider.getProxyImplementation(
-      convertibleAddressId
-    );
-    expect(implementationAddressAfter).to.be.eq(ZERO_ADDRESS);
+    await expect(getProxyImplementation(addressesProvider.address, registeredAddress)).to.be
+      .reverted;
   });
 
   it('Owner updates the implementation of a proxy which is already initialized', async () => {
@@ -262,7 +266,12 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
     expect(poolAddress).to.be.not.eq(ZERO_ADDRESS);
 
     const poolAddressId = utils.formatBytes32String('POOL');
-    const implementationAddress = await addressesProvider.getProxyImplementation(poolAddressId);
+    const proxyAddress = await addressesProvider.getAddress(poolAddressId);
+    const implementationAddress = await getProxyImplementation(
+      addressesProvider.address,
+      proxyAddress
+    );
+
     // Update the Pool proxy
     expect(
       await addressesProvider
@@ -315,8 +324,10 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
     expect(await addressesProvider.getPoolConfigurator(), configurator.address);
 
     const poolConfiguratorAddressId = utils.formatBytes32String('POOL_CONFIGURATOR');
-    const implementationAddress = await addressesProvider.getProxyImplementation(
-      poolConfiguratorAddressId
+    const proxyAddress = await addressesProvider.getAddress(poolConfiguratorAddressId);
+    const implementationAddress = await getProxyImplementation(
+      addressesProvider.address,
+      proxyAddress
     );
 
     expect(
@@ -328,12 +339,12 @@ makeSuite('PoolAddressesProvider', (testEnv: TestEnv) => {
       .withArgs(implementationAddress, newPoolConfiguratorImpl);
 
     expect(await addressesProvider.getPoolConfigurator()).to.be.eq(configurator.address);
-    expect(await addressesProvider.getProxyImplementation(poolConfiguratorAddressId)).to.be.not.eq(
-      implementationAddress
-    );
-    expect(await addressesProvider.getProxyImplementation(poolConfiguratorAddressId)).to.be.eq(
-      newPoolConfiguratorImpl
+    const implementationAddressAfter = await getProxyImplementation(
+      addressesProvider.address,
+      proxyAddress
     );
+    expect(implementationAddressAfter).to.be.not.eq(implementationAddress);
+    expect(implementationAddressAfter).to.be.eq(newPoolConfiguratorImpl);
 
     await evmRevert(snapId);
   });

test-suites/pool-edge.spec.ts

@@ -16,6 +16,7 @@ import {
   VariableDebtToken__factory,
   AToken__factory,
   Pool__factory,
+  InitializableImmutableAdminUpgradeabilityProxy,
 } from '../types';
 
 declare var hre: HardhatRuntimeEnvironment;
@@ -324,7 +325,18 @@ makeSuite('Pool: Edge cases', (testEnv: TestEnv) => {
       log: false,
     });
 
-    const oldPoolImpl = await addressesProvider.getProxyImplementation(POOL_ID);
+    // Impersonate PoolAddressesProvider
+    await impersonateAccountsHardhat([addressesProvider.address]);
+    const addressesProviderSigner = await hre.ethers.getSigner(addressesProvider.address);
+
+    const poolProxyAddress = await addressesProvider.getPool();
+    const poolProxy = (await hre.ethers.getContractAt(
+      'InitializableImmutableAdminUpgradeabilityProxy',
+      poolProxyAddress,
+      addressesProviderSigner
+    )) as InitializableImmutableAdminUpgradeabilityProxy;
+
+    const oldPoolImpl = await poolProxy.callStatic.implementation();
 
     // Upgrade the Pool
     expect(
@@ -492,7 +504,18 @@ makeSuite('Pool: Edge cases', (testEnv: TestEnv) => {
       log: false,
     });
 
-    const implementationAddress = await addressesProvider.getProxyImplementation(POOL_ID);
+    // Impersonate PoolAddressesProvider
+    await impersonateAccountsHardhat([addressesProvider.address]);
+    const addressesProviderSigner = await hre.ethers.getSigner(addressesProvider.address);
+
+    const proxyAddress = await addressesProvider.getAddress(POOL_ID);
+    const proxy = (await hre.ethers.getContractAt(
+      'InitializableImmutableAdminUpgradeabilityProxy',
+      proxyAddress,
+      addressesProviderSigner
+    )) as InitializableImmutableAdminUpgradeabilityProxy;
+
+    const implementationAddress = await proxy.callStatic.implementation();
 
     // Upgrade the Pool
     expect(