External interop poc #879

batconjurer · 2025-11-04T16:19:42Z

This draft PR is to give visibility to the POC for bridging avalanche to ethereum

…o making sure commits are signed correctly :)

github-advanced-security

Semgrep PRO found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Signed-off-by: Jacob Turner <[email protected]>

…thereum

geoff-vball · 2025-11-17T15:04:11Z

avalanche/contracts/teleporter/TeleporterMessenger.sol

 import {ITeleporterReceiver} from "./ITeleporterReceiver.sol";
 import {ReentrancyGuards} from "@utilities/ReentrancyGuards.sol";
+import {IWarpExt} from "./IWarpExt.sol";
+import {IWarpMessenger} from "../../lib/subnet-evm/contracts/contracts/interfaces/IWarpMessenger.sol";


Just a heads up, in main we've removed the subnet-evm submodule in favor of just copying over the 3 contracts we use from there.

avalanche/contracts/teleporter/TeleporterMessenger.sol

geoff-vball · 2025-11-17T15:39:29Z

tests/utils/teleporter.go

 package utils

 import (
-	"bytes"


Looks like your IDE went and replaced the tabs with spaces in this files and teleporter_utils. Tbh I didn't even know we used tabs, but definitely don't want the entire file in the diff

Yeah, I noticed that. Sigh. Although it is much better to have spaces than tabs, that is definitely not something we should do in this PR. The IDE is currently set to autoformat on file save and I guess that did it

I used to be in the spaces camp but gofmt uses tabs and we should do the same.

Ok, I've adjusted the settings in my IDE and hopefully that fixes the issue.

geoff-vball · 2025-11-18T15:38:56Z

ethereum/contracts/AvalancheValidatorSetRegistry.sol

+        uint256 validatorSetID = nextValidatorSetID++;
+        _validatorSets[validatorSetID] = validatorSet;


Is there a reason you chose a monotonic counter here instead of using something like the avalancheBlockChainID or a hash of avalancheNetworkID, avalancheBlockChainID?

I inherited this decision, but in its defense, it makes finding the latest n validator sets very easy. This is nice for updating the current validator set and for only validating sufficiently fresh messages.

Also, these are all the validators for a specific L1, so they can't be keyed by avalancheNetworkID, avalancheBlockChainID, etc as those will be constant

I guess the current setup allows registering the validators of any given L1 multiple times, whereas keying on avalancheBlockChainID would require a single set

tests/utils/avalanche_validator_registry.go

tests/utils/eth_warp.go

Co-authored-by: Geoff Stuart <[email protected]> Signed-off-by: Jacob Turner <[email protected]>

ethereum/contracts/EthWarp.sol

geoff-vball · 2025-11-19T15:20:46Z

ethereum/contracts/utils/ICM.sol

+            sourceChainID: message.avalancheSourceBlockchainID,
+            // N.B. This prevents this function from being used for internal interop calls
+            // to teleporter
+            originSenderAddress: address(0),


I think we need to use the actual sender address here (eventually). We'll need a way of initializing this contract with the address of teleporter v2 on Avalanche. If we don't check that the message was emitted by teleporter v2, any contract on Avalanche could spoof any other contract on the same source blockchain.

So I'm not sure about this. It may not be clear from this function's location (which can be fixed), but this is called only by EthWarp (which is called directly by teleporter) after checking for a quorum of signatures. So I think we trust any message signed by a quorum of signatures, regardless of origin.

So yes, we've checked that the message was signed by a quorum of signatures, but we don't know what contract sent the message. The reason we use Nick's method to deploy Teleporter to the same address on Avalanche is so that on the receiving side, we can know exactly what code emitted the warp message.

As-is, a malicious contract on the same chain could emit a warp message that exactly spoofs any other Teleporter message. Basically the way this contract will (probably) have to work is that we'll create the Avalanche Teleporter V2 contract, deploy via Nick's method, and then hard-code that address here so we can verify which contract emitted the message.

ethereum/contracts/utils/ByteComparator.sol

geoff-vball · 2025-11-19T17:52:59Z

ethereum/contracts/utils/ByteSlicer.sol

+        for (uint256 i = 0; i < length; i++) {
+            target[cursor + i] = source[i];
+        }
+        return (target, cursor + length);


If the new ending position is deterministic, we don't really need to calculate and return it here

indeed, it was just to keep the function calling this clean.

ethereum/contracts/utils/ICM.sol

geoff-vball · 2025-11-19T18:10:37Z

ethereum/contracts/utils/ICM.sol

+        // TODO: Do we need to check the avalanche source blockchain ID?
+        // It's expected to be different in cases where the message is from the primary network.
+        // if (
+        //     message.unsignedMessage.avalancheSourceBlockchainID
+        //         != validatorSet.avalancheBlockchainID
+        // ) {
+        //     revert("Invalid avalanche source blockchain ID");
+        // }


Yeah, we definitely need to check the source blockchain ID to know which validator set to compare it against (if we want to allow paths other than Eth <-> C-chain)

So there is a subtlety here. This function needs to verify two different types of messages: normal teleporter messages and validator set updates. In the first instance, this check is done at a higher scope by the calling EthWarp contract. For the second, EthWarp is bypassed altogether since it is outside the scope of teleporter. In the e2e test, the initial validator set is actually signed over by the P-chain, which would cause this check to fail. Hence the comment

Got it. So for validator set updates, message.unsignedMessage.avalancheSourceBlockchainID would be the p-chain, but validatorSet.avalancheBlockchainID would be the blockchainID of the validator set being updated.

geoff-vball · 2025-11-19T18:12:28Z

ethereum/contracts/utils/ICM.sol

+        if (!result) {
+            revert("Invalid signature");
+        }
+        return result;


It seems strange to have both reverts and a boolean return value. We can go with one or the other

Hah! I hadn't even noticed that

Co-authored-by: Geoff Stuart <[email protected]> Signed-off-by: Jacob Turner <[email protected]>

geoff-vball · 2025-11-21T01:12:54Z

ethereum/contracts/EthWarp.sol

+     */
+    function registerChain(bytes32 avaBlockchainId, address verifyWarpMessage) external {
+        require(!isChainRegistered(avaBlockchainId), "This chain is already registered");
+        require(verifyWarpMessage != address(0), "Provided address does not exist");


No need for a change yet, but the way we implemented registering contracts in ICTT iirc is that we had a register method on the given contract (so on the verifyWarpMessage contract in this case) so that we could at least be sure that the address supplied corresponded to a contract with the register method and wasn't incorrectly input (though someone could create a malicious dummy contract and point to that).

geoff-vball · 2025-11-21T14:30:23Z

ethereum/contracts/utils/ICM.sol

+    function bytesToBoolArray(
+        bytes memory data
+    ) internal pure returns (bool[] memory) {


I'm not sure if its feasible, but it may be more efficient to operate on the bitset directly instead of transforming it into a bool array (can be a future investigation)

ethereum/contracts/utils/ICM.sol

geoff-vball · 2025-11-21T15:43:11Z

ethereum/contracts/utils/ICM.sol

+        // TODO: Do we need to check the avalanche source blockchain ID?
+        // It's expected to be different in cases where the message is from the primary network.
+        // if (
+        //     message.unsignedMessage.avalancheSourceBlockchainID
+        //         != validatorSet.avalancheBlockchainID
+        // ) {
+        //     revert("Invalid avalanche source blockchain ID");
+        // }


Got it. So for validator set updates, message.unsignedMessage.avalancheSourceBlockchainID would be the p-chain, but validatorSet.avalancheBlockchainID would be the blockchainID of the validator set being updated.

geoff-vball · 2025-11-21T15:47:03Z

ethereum/contracts/utils/ICM.sol

+            sourceChainID: message.avalancheSourceBlockchainID,
+            // N.B. This prevents this function from being used for internal interop calls
+            // to teleporter
+            originSenderAddress: address(0),


So yes, we've checked that the message was signed by a quorum of signatures, but we don't know what contract sent the message. The reason we use Nick's method to deploy Teleporter to the same address on Avalanche is so that on the receiving side, we can know exactly what code emitted the warp message.

As-is, a malicious contract on the same chain could emit a warp message that exactly spoofs any other Teleporter message. Basically the way this contract will (probably) have to work is that we'll create the Avalanche Teleporter V2 contract, deploy via Nick's method, and then hard-code that address here so we can verify which contract emitted the message.

geoff-vball · 2025-11-21T15:50:44Z

ethereum/contracts/utils/ValidatorSets.sol

+            require(
+                ByteComparator.compare(unformattedPublicKey, previousPublicKey) > 0,
+                "BLS public key must be greater than the latest public key"
+            );
+            uint64 weight = uint64(bytes8(ByteSlicer.slice(data, offset + 96, 8)));
+            require(weight > 0, "Validator weight must be greater than 0");


For now best to keep these checks, but we can probably assume if the message is signed by a quorum of validators that it is indeed valid.

geoff-vball · 2025-11-21T15:55:57Z

ethereum/contracts/utils/ValidatorSets.sol

+        return ValidatorSetStatePayload({
+            avalancheBlockchainID: avalancheBlockchainID,
+            pChainHeight: pChainHeight,
+            pChainTimestamp: pChainTimestamp,
+            validatorSetHash: validatorSetHash
+        });


Is this a message that the p-chain has to sign?

That's the way it currently is. In theory, the L1 validator set should be sufficient except for maybe the first message.

geoff-vball · 2025-11-21T15:59:00Z

tests/flows/ethereum-icm-verification/README.md

+1. Creates an Avalanche L1 on the Fuji testnet with 3 local validators, running a custom version of AvalancheGo.
+1. Deploys an `AvalancheValidatorSetRegistry` contract on the Sepolia testnet.
+1. Registers the Avalanche L1's validator set in the registry via an ICM message from the P-Chain, self-signed by the validator set of the L1.
+1. Sends a transaction on the L1 to send an ICM message to Sepolia.
+1. Gets an aggregate signature of the ICM message using the ICM signature aggregator.
+1. Broadcasts the signed ICM message to Sepolia, and confirms successful inclusion and verification.
+1. Adds a new validator to the L1, representing 40% of the new total weight.
+1. Updates the validator set reigstered on the Sepolia registry via a validator set update message signed by sufficient weight of the previous validator set.
+1. Sends and verifies another ICM message from the L1 to Sepolia.


Numbering here is broken

mdelle1 · 2025-11-21T19:02:54Z

I was wondering if this PR could be broken up into multiple smaller, standalone PRs? The breadth of the PR makes it a bit harder to review

Co-authored-by: Geoff Stuart <[email protected]> Signed-off-by: Jacob Turner <[email protected]>

batconjurer · 2025-11-24T14:35:57Z

I was wondering if this PR could be broken up into multiple smaller, standalone PRs? The breadth of the PR makes it a bit harder to review

Yes, I'm doing that now.

…contract

michaelkaplan13 and others added 9 commits July 17, 2025 09:41

Restructure

a34b0b9

save

d705a68

E2E test set up and log fix

ca2d534

working e2e test

33d00bf

Clean up

3ec202d

Update TODOs

fb32a42

Add Warp adapters for validating messages on Ethereum. Needs testing

2d07682

Added a test that validator set registry enforces the network id. Als…

1a7fa48

…o making sure commits are signed correctly :)

Adding unit tests for verifying warp messages on Ethereum

401e993

batconjurer requested a review from a team as a code owner November 4, 2025 16:19

batconjurer requested review from bernard-avalabs, geoff-vball, iansuvak, mdelle1, michaelkaplan13 and ylg-avalabs November 4, 2025 16:19

github-advanced-security bot found potential problems Nov 4, 2025

View reviewed changes

batconjurer marked this pull request as draft November 4, 2025 16:20

batconjurer added 8 commits November 5, 2025 10:52

Small cleanup

30f820b

Signed-off-by: Jacob Turner <[email protected]>

Fixed unit tests and added to the Ethereum ICM e2e test

e005e00

small cleanups

7d8c63b

small cleanups

06a43f4

Cleanup: remove unneeded field from EthWarp contract

dcbddd4

Fleshed out e2e test

13fce4f

Fleshed out e2e test

5277257

Strengthen checks for checking an ICM message was correctly sent to E…

bfe38dd

…thereum

geoff-vball reviewed Nov 17, 2025

View reviewed changes

avalanche/contracts/teleporter/TeleporterMessenger.sol Outdated Show resolved Hide resolved

geoff-vball reviewed Nov 17, 2025

View reviewed changes

Fixed the packing of ReceiveInterChainMessage calls

25a32e1

geoff-vball reviewed Nov 18, 2025

View reviewed changes

switch from spaces to tabs

b740bfd

geoff-vball reviewed Nov 19, 2025

View reviewed changes

tests/utils/avalanche_validator_registry.go Outdated Show resolved Hide resolved

tests/utils/avalanche_validator_registry.go Outdated Show resolved Hide resolved

tests/utils/avalanche_validator_registry.go Outdated Show resolved Hide resolved

tests/utils/eth_warp.go Show resolved Hide resolved

batconjurer and others added 3 commits November 19, 2025 11:15

Update tests/utils/eth_warp.go

74958a6

Co-authored-by: Geoff Stuart <[email protected]> Signed-off-by: Jacob Turner <[email protected]>

Addressing PR comments regarding e2e logging and error handling

cf96fb4

Refactor TeleporterMessenger contract to use initializer modifier

aa36665

geoff-vball reviewed Nov 19, 2025

View reviewed changes

ethereum/contracts/EthWarp.sol Outdated Show resolved Hide resolved

geoff-vball reviewed Nov 19, 2025

View reviewed changes

ethereum/contracts/utils/ByteComparator.sol Outdated Show resolved Hide resolved

geoff-vball reviewed Nov 19, 2025

View reviewed changes

ethereum/contracts/utils/ByteComparator.sol Outdated Show resolved Hide resolved

geoff-vball reviewed Nov 19, 2025

View reviewed changes

ethereum/contracts/utils/ICM.sol Outdated Show resolved Hide resolved

geoff-vball reviewed Nov 19, 2025

View reviewed changes

ethereum/contracts/utils/ICM.sol Outdated Show resolved Hide resolved

geoff-vball reviewed Nov 19, 2025

View reviewed changes

batconjurer and others added 2 commits November 20, 2025 10:45

Update ethereum/contracts/utils/ICM.sol

0749ba3

Co-authored-by: Geoff Stuart <[email protected]> Signed-off-by: Jacob Turner <[email protected]>

Addressing PR comments

4dce051

geoff-vball reviewed Nov 21, 2025

View reviewed changes

Update ethereum/contracts/utils/ICM.sol

8caaea7

Co-authored-by: Geoff Stuart <[email protected]> Signed-off-by: Jacob Turner <[email protected]>

Enforce the blockchain ID validation in AvalancheValidatorSetRegisty …

6ead19f

…contract

		uint256 validatorSetID = nextValidatorSetID++;
		_validatorSets[validatorSetID] = validatorSet;

External interop poc #879

Are you sure you want to change the base?

External interop poc #879

Uh oh!

Conversation

batconjurer commented Nov 4, 2025

Uh oh!

github-advanced-security bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdelle1 commented Nov 21, 2025

Uh oh!

batconjurer commented Nov 24, 2025

Uh oh!

Reviewers