Merge branch 'feat/ven-1918' into feat/omnichain-governance-deployments

contracts/Cross-chain/OmnichainExecutorOwner.sol

@@ -3,6 +3,7 @@ pragma solidity 0.8.25;
 
 import { AccessControlledV8 } from "../Governance/AccessControlledV8.sol";
 import { IOmnichainGovernanceExecutor } from "./interfaces/IOmnichainGovernanceExecutor.sol";
+import { ensureNonzeroAddress } from "@venusprotocol/solidity-utilities/contracts/validators.sol";
 
 /**
  * @title OmnichainExecutorOwner
@@ -45,6 +46,21 @@ contract OmnichainExecutorOwner is AccessControlledV8 {
         __AccessControlled_init(accessControlManager_);
     }
 
+    /**
+     * @notice Sets the source message sender address
+     * @param srcChainId_ The LayerZero id of a source chain
+     * @param srcAddress_ The address of the contract on the source chain
+     * @custom:access Controlled by AccessControlManager
+     * @custom:event Emits SetTrustedRemoteAddress with source chain Id and source address
+     */
+    function setTrustedRemoteAddress(uint16 srcChainId_, bytes calldata srcAddress_) external {
+        _checkAccessAllowed("setTrustedRemoteAddress(uint16,bytes)");
+        require(srcChainId_ != 0, "ChainId must not be zero");
+        ensureNonzeroAddress(address(uint160(bytes20(srcAddress_))));
+        require(srcAddress_.length == 20, "Source address must be 20 bytes long");
+        OMNICHAIN_GOVERNANCE_EXECUTOR.setTrustedRemoteAddress(srcChainId_, srcAddress_);
+    }
+
     /**
      * @notice Invoked when called function does not exist in the contract
      * @param data_ Calldata containing the encoded function call

contracts/Cross-chain/OmnichainGovernanceExecutor.sol

@@ -278,6 +278,10 @@ contract OmnichainGovernanceExecutor is ReentrancyGuard, BaseOmnichainController
         uint64 nonce_,
         bytes calldata payload_
     ) public payable override onlyOwner nonReentrant {
+        require(
+            keccak256(trustedRemoteLookup[srcChainId_]) == keccak256(srcAddress_),
+            "OmnichainGovernanceExecutor::retryMessage: not a trusted remote"
+        );
         super.retryMessage(srcChainId_, srcAddress_, nonce_, payload_);
     }
 

contracts/Cross-chain/OmnichainProposalSender.sol

@@ -122,7 +122,7 @@ contract OmnichainProposalSender is ReentrancyGuard, BaseOmnichainControllerSrc
      * @param payload_ The payload to be sent to the remote chain
      * It's computed as follows: payload = abi.encode(targets, values, signatures, calldatas, proposalType)
      * @param adapterParams_ The params used to specify the custom amount of gas required for the execution on the destination
-     * @param zroPaymentAddress_ The address of the ZRO token holder who would pay for the transaction
+     * @param zroPaymentAddress_ The address of the ZRO token holder who would pay for the transaction. This must be either address(this) or tx.origin
      * @custom:event Emits ExecuteRemoteProposal with remote chain id, proposal ID and payload on success
      * @custom:event Emits StorePayload with last stored payload proposal ID ,remote chain id , payload, adapter params , values and reason for failure
      * @custom:access Controlled by Access Control Manager
@@ -263,6 +263,7 @@ contract OmnichainProposalSender is ReentrancyGuard, BaseOmnichainControllerSrc
         _ensureAllowed("setTrustedRemoteAddress(uint16,bytes)");
         require(remoteChainId_ != 0, "OmnichainProposalSender: chainId must not be zero");
         ensureNonzeroAddress(address(uint160(bytes20(newRemoteAddress_))));
+        require(newRemoteAddress_.length == 20, "OmnichainProposalSender: remote address must be 20 bytes long");
         bytes memory oldRemoteAddress = trustedRemoteLookup[remoteChainId_];
         trustedRemoteLookup[remoteChainId_] = abi.encodePacked(newRemoteAddress_, address(this));
         emit SetTrustedRemoteAddress(remoteChainId_, oldRemoteAddress, trustedRemoteLookup[remoteChainId_]);

contracts/Cross-chain/interfaces/IOmnichainGovernanceExecutor.sol

@@ -7,4 +7,11 @@ interface IOmnichainGovernanceExecutor {
      * @param addr The address to which ownership will be transferred
      */
     function transferOwnership(address addr) external;
+
+    /**
+     * @notice Sets the source message sender address
+     * @param srcChainId_ The LayerZero id of a source chain
+     * @param srcAddress_ The address of the contract on the source chain
+     */
+    function setTrustedRemoteAddress(uint16 srcChainId_, bytes calldata srcAddress_) external;
 }

tests/Cross-chain/Omnichain.ts

@@ -81,7 +81,6 @@ describe("Omnichain: ", async function () {
       "setSendVersion(uint16)",
       "setReceiveVersion(uint16)",
       "forceResumeReceive(uint16,bytes)",
-      "setTrustedRemoteAddress(uint16,bytes)",
       "setPrecrime(address)",
       "setMinDstGas(uint16,uint16,uint256)",
       "setPayloadSizeLimit(uint16,uint256)",
@@ -261,6 +260,12 @@ describe("Omnichain: ", async function () {
     ).to.be.revertedWith("OmnichainProposalSender: destination chain is not a trusted source");
   });
 
+  it("Reverts if remote address is more than 20 bytes", async function () {
+    await expect(sender.connect(signer1).setTrustedRemoteAddress(remoteChainId, remotePath + "99")).to.be.revertedWith(
+      "OmnichainProposalSender: remote address must be 20 bytes long",
+    );
+  });
+
   it("Reverts with Daily Transaction Limit Exceed", async function () {
     await sender.connect(signer1).setMaxDailyLimit(remoteChainId, 0);
     const payload = await getPayload(NormalTimelock.address);
@@ -301,9 +306,22 @@ describe("Omnichain: ", async function () {
   });
 
   it("Function registry should not emit event if function is added twice", async function () {
-    expect(
-      await executorOwner.connect(deployer).upsertSignature(["setTrustedRemoteAddress(uint16,bytes)"], [true]),
-    ).to.not.emit(executorOwner, "FunctionRegistryChanged");
+    await expect(executorOwner.connect(deployer).upsertSignature(["pause()"], [true])).to.not.emit(
+      executorOwner,
+      "FunctionRegistryChanged",
+    );
+  });
+
+  it("Reverts if invalid parameters passed in trusted remote", async function () {
+    await expect(executorOwner.connect(signer1).setTrustedRemoteAddress(0, localPath)).to.be.revertedWith(
+      "ChainId must not be zero",
+    );
+    await expect(
+      executorOwner.connect(signer1).setTrustedRemoteAddress(localChainId, ethers.constants.AddressZero),
+    ).to.be.revertedWithCustomError(executor, "ZeroAddressNotAllowed");
+    await expect(
+      executorOwner.connect(signer1).setTrustedRemoteAddress(localChainId, localPath + "99"),
+    ).to.be.revertedWith("Source address must be 20 bytes long");
   });
 
   it("Reverts if EOA called owner function of Executor", async function () {
@@ -608,6 +626,50 @@ describe("Omnichain: ", async function () {
     expect(await executor.failedMessages(localChainId, srcAddress, 1)).equals(ethers.constants.HashZero);
   });
 
+  it("Revert retry message on destination if trusted remote is changed", async function () {
+    let data = executor.interface.encodeFunctionData("pause", []);
+    const srcAddress = ethers.utils.solidityPack(["address", "address"], [sender.address, executor.address]);
+    await expect(
+      signer1.sendTransaction({
+        to: executorOwner.address,
+        data: data,
+      }),
+    ).to.emit(executor, "Paused");
+    const payload = await getPayload(NormalTimelock.address);
+    expect(
+      await sender.connect(signer1).execute(remoteChainId, payload, adapterParams, ethers.constants.AddressZero, {
+        value: nativeFee,
+      }),
+    ).to.emit(sender, "ExecuteRemoteProposal");
+    expect(await executor.failedMessages(localChainId, srcAddress, 1)).not.equals(ethers.constants.HashZero);
+
+    data = executor.interface.encodeFunctionData("unpause", []);
+    await expect(
+      signer1.sendTransaction({
+        to: executorOwner.address,
+        data: data,
+      }),
+    ).to.emit(executor, "Unpaused");
+
+    const addressOne = "0x0000000000000000000000000000000000000001";
+
+    //change trusted remote
+    await executorOwner.connect(signer1).setTrustedRemoteAddress(localChainId, addressOne);
+
+    data = executor.interface.encodeFunctionData("retryMessage", [
+      localChainId,
+      srcAddress,
+      1,
+      await payloadWithId(payload),
+    ]);
+    await expect(
+      signer1.sendTransaction({
+        to: executorOwner.address,
+        data: data,
+      }),
+    ).to.be.reverted;
+  });
+
   it("Retry messages that failed due to low gas at the destination using the Endpoint.", async function () {
     // low destination gas
     const adapterParamsLocal = ethers.utils.solidityPack(["uint16", "uint256"], [remoteChainId, 10000]);