Ethereum bridge milestone 2: implement execute_signed() by side2k · Pull Request #506 · fpco/kolme

side2k · 2026-03-04T11:49:07Z

Depends on #505

add helpers for cryptographic validation of processor and approvers' signatures
implement execute_signed(payload, processor, approvers) entry point in Ethereum bridge contract
- decodes payload as (action_id, action_data)
  - at this point, action_data is ignored, will be implemented in next milestone
- validates action_id ordering against nextActionId
- verifies processor signature over sha256(payload)
- verifies approver signatures (membership, uniqueness, quorum threshold)
- emits Signed(event_id, sender, action_id) and increments nextEventId / nextActionId
precompute/store processor + approver derived EVM signer addresses in contract state during constructor init to make runtime signature checks straightforward
test coverage
- for cryptographic helpers
- for execute_signed() base functionality
- smoke tests for the contract, including execute_signed() testing
  - this one added as separate step into CI - to be removed later, when submitter tests covering that are implemented

cloudflare-workers-and-pages · 2026-03-04T11:55:52Z

Deploying kolme with Cloudflare Pages

Latest commit:	`5dbea27`
Status:	✅ Deploy successful!
Preview URL:	https://3d75ae84.kolme.pages.dev
Branch Preview URL:	https://ethereum-bridge-2-execute.kolme.pages.dev

View logs

It will be used to verify processor's and approvers' signatures

To be used in conjunction with _recoverSigner() for signature verification

…dress() match

…state

…/approver keys

...and run them in CI

snoyberg · 2026-03-05T10:33:09Z

contracts/ethereum/src/Bridge.sol

    ValidatorSet internal validatorSet;
    uint64 internal nextEventId;
    uint64 internal nextActionId;
+    address internal processorSigner;


Adding these fields duplicates storage and additionally introduces potential bugs when rotating signers. I'd rather instead that we compute these as needed. (I can't speak to Ethereum APIs and whether such a conversion is strictly necessary.)

Fixed - moved those calculations into the execute_signed(). This will increase gas usage somewhat (not significantly, as far as I can tell by looking at a test_ExecuteSignedIncrementsIdsAndEmitsEvent() gas usage (116k vs 101k). Do you think its worth to investigate gas usage more precisely here?

No, let's leave it alone for the moment, we can worry about optimizations like that later.

snoyberg · 2026-03-05T10:37:50Z

contracts/ethereum/src/Bridge.sol

    uint64 internal nextEventId;
    uint64 internal nextActionId;
+    address internal processorSigner;
+    mapping(address => bool) internal approverSigners;


Is there a reason that here you used a mapping but in ValidatorSet the approvers are represented as an array?

I got to know about mapping existence a bit later, and thought about going there, but in this particular case mapping is only useful if we put EVM addresses in it, and if we convert validator keys into those, there's no reverse conversion - so we'll lose original key values.

Copilot

Pull request overview

Implements milestone 2 of the Ethereum bridge by adding signature verification helpers and introducing the execute_signed(payload, processorSig, approverSigs) entry point, plus CI/e2e smoke coverage to validate the on-chain flow.

Changes:

Add execute_signed() to the bridge contract with action-id ordering + processor/approver signature verification and a new Signed event.
Derive and store processor/approver EVM signer addresses at construction time to simplify runtime signature checks.
Expand Solidity + Rust integration tests and add a Dockerized Anvil CLI smoke test step in CI.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`contracts/ethereum/src/Bridge.sol`	Adds `execute_signed`, signature recovery, pubkey→address derivation, new errors, and signer state.
`contracts/ethereum/test/Bridge.t.sol`	Adds harness + tests for signature helpers and `execute_signed` happy/negative paths.
`contracts/ethereum/script/Bridge.s.sol`	Updates default deployment keys to distinct processor/approver keys.
`packages/integration-tests/tests/ethereum-bridge.rs`	Updates expected config keys to match deterministic e2e deployment values.
`contracts/ethereum/e2e/ethereum-anvil-cli-smoke.sh`	New CLI smoke script exercising `get_config` + `execute_signed` and replay rejection.
`contracts/ethereum/e2e/compose.yaml`	Mounts the new smoke script into the Anvil container.
`contracts/ethereum/e2e/bridge/bootstrap-state.sh`	Updates deterministic deployment keys and improves bridge address extraction.
`contracts/ethereum/e2e/bridge/Dockerfile`	Pins Foundry install version and includes tooling needed for smoke tests (e.g., `jq`).
`justfile`	Adds a `just ethereum-anvil-cli-smoke` recipe in the e2e working directory.
`.github/workflows/ci.yml`	Adds the Anvil CLI smoke step to CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

contracts/ethereum/e2e/ethereum-anvil-cli-smoke.sh

Copilot · 2026-03-05T10:50:55Z

contracts/ethereum/test/Bridge.t.sol

+        bytes[] memory approverSigs = new bytes[](1);
+        approverSigs[0] = _signPayload(APPROVER_PRIVATE_KEY, payload);
+
+        vm.expectEmit(true, false, false, true, address(bridge));


expectEmit(true, false, false, true, ...) does not verify the indexed sender topic for Signed. Since this test already controls the caller with vm.prank, it can safely enable checking topic2 as well to make the assertion stricter and catch accidental changes to emitted sender.

Suggested change

vm.expectEmit(true, false, false, true, address(bridge));

vm.expectEmit(true, true, false, true, address(bridge));

contracts/ethereum/src/Bridge.sol

Copilot · 2026-03-05T10:50:56Z

contracts/ethereum/src/Bridge.sol

+        processorSigner = _validatorAddress(processor);
+        for (uint256 i = 0; i < approvers.length; i++) {
+            approverSigners[_validatorAddress(approvers[i])] = true;
+        }


The constructor derives EVM addresses (and therefore validates curve points) for the processor and approvers, but it does not do the same for listeners. This leaves the contract able to deploy with listener keys that are valid-length/prefix but invalid secp256k1 points, which will likely break later milestones once listener signature verification is added. Consider validating all validator keys’ curve points during construction (or explicitly document why listeners are exempt for now).

Copilot · 2026-03-05T10:50:56Z

contracts/ethereum/src/Bridge.sol

+        uint256 approverCount = approvers.length;
+        address[] memory seen = new address[](approverCount);
+        uint256 uniqueApprovers = 0;
+        for (uint256 i = 0; i < approverCount; i++) {
+            address signer = _recoverSigner(payloadHash, approvers[i]);
+            if (!approverSigners[signer]) {
+                revert InvalidApproverSignature(signer);
+            }
+            for (uint256 j = 0; j < uniqueApprovers; j++) {
+                if (seen[j] == signer) {
+                    revert DuplicateApproverSignature(signer);
+                }
+            }
+            seen[uniqueApprovers] = signer;
+            uniqueApprovers += 1;
+        }


execute_signed performs an O(n^2) duplicate check across all provided approver signatures, and it will iterate even if the caller supplies far more signatures than the configured approver set. Add a cheap bound/early-exit (e.g., revert if approvers.length exceeds the configured total, and/or stop once neededApprovers unique valid signatures are collected) to keep gas costs predictable.

Partially addressed - added early exit when the quorum is reached

Copilot · 2026-03-05T10:50:56Z

contracts/ethereum/src/Bridge.sol

+        bool success;
+        assembly {
+            success := staticcall(
+                gas(),
+                0x05,
+                add(input, 0x20),
+                mload(input),
+                add(output, 0x20),
+                0x20
+            )
+        }
+        assert(success);
+        return abi.decode(output, (uint256));


_modExp uses assert(success). If the precompile call ever fails, this triggers a Panic and consumes all remaining gas, which is undesirable for a public contract. Prefer a normal revert with a dedicated error (and optionally include the inputs) so failures are debuggable and don’t burn all gas.

Copilot · 2026-03-05T10:50:57Z

contracts/ethereum/e2e/bridge/Dockerfile

 RUN curl -L https://foundry.paradigm.xyz | bash && \
-    /root/.foundry/bin/foundryup
+    /root/.foundry/bin/foundryup --install "${FOUNDRY_VERSION}"


curl -L https://foundry.paradigm.xyz | bash downloads and executes remote code during the image build without any integrity verification, so a compromise or MITM of that endpoint could run arbitrary commands in your build/CI environment and tamper with artifacts or exfiltrate secrets. To mitigate this, avoid piping curl directly to bash and instead download a specific, integrity-checked artifact (pinned by checksum, signature, or content hash) before executing it.

I'd leave it as is for now

…tate

snoyberg · 2026-03-05T12:15:31Z

contracts/ethereum/src/Bridge.sol

+        }
+
        uint256 approverCount = approvers.length;
        address[] memory seen = new address[](approverCount);


Can this be a mapping to avoid the extra for loop below to check if something has been seen?

mapping can be used only in storage, if I understand it correctly

https://docs.soliditylang.org/en/latest/types.html#mapping-types

…t in constructor

…ched

…entsIdsAndEmitsEvent()

side2k changed the title ~~Ethereum bridge milestone 2: implement execute()~~ WIP: Ethereum bridge milestone 2: implement execute() Mar 4, 2026

side2k changed the title ~~WIP: Ethereum bridge milestone 2: implement execute()~~ WIP: Ethereum bridge milestone 2: implement execute_signed() Mar 4, 2026

side2k force-pushed the ethereum-bridge-2-execute branch from b399144 to 83ae599 Compare March 4, 2026 13:16

Base automatically changed from ethereum-bridge-1-5-get-resume-cursor to main March 5, 2026 09:19

Leonid Amirov added 11 commits March 5, 2026 12:19

feat(ethereum): contract: implement _recoverSigner() helper and tests

1268d73

It will be used to verify processor's and approvers' signatures

feat(ethereum): contract: implement _validatorAddress() helper and tests

9ec9fbe

To be used in conjunction with _recoverSigner() for signature verification

feat(tests): ethereum bridge: cover _recoverSigner() and _validatorAd…

5ebba4e

…dress() match

feat(ethereum): contract: store precomputed signers EVM addresses in …

b7752f5

…state

feat(ethereum): contract: implement base execute()

1745507

chore: formatting in contracts/ethereum/src/Bridge.sol

79450cf

feat(tests): ethereum contract: cover execute()

18f3228

chore(ethereum): contract: update deployment script default processor…

a4493c5

…/approver keys

feat(tests): add smoke tests for ethereum contract's execute()

561b5a6

...and run them in CI

chore(ethereum): contract: rename execute() to execute_signed()

78104e9

chore(tests): ethereum e2e tests: pin Foundry version

9e07ba5

side2k force-pushed the ethereum-bridge-2-execute branch from 83ae599 to 9e07ba5 Compare March 5, 2026 09:32

side2k changed the title ~~WIP: Ethereum bridge milestone 2: implement execute_signed()~~ Ethereum bridge milestone 2: implement execute_signed() Mar 5, 2026

side2k marked this pull request as ready for review March 5, 2026 09:32

side2k requested a review from snoyberg March 5, 2026 09:32

snoyberg requested a review from Copilot March 5, 2026 10:39

Copilot started reviewing on behalf of snoyberg March 5, 2026 10:40 View session

snoyberg requested changes Mar 5, 2026

View reviewed changes

Copilot AI reviewed Mar 5, 2026

View reviewed changes

feat(ethereum): contract: do not store precomputed approvers in EVM s…

cd76fb7

…tate

side2k requested a review from snoyberg March 5, 2026 11:59

snoyberg approved these changes Mar 5, 2026

View reviewed changes

Leonid Amirov added 3 commits March 5, 2026 15:20

feat(ethereum): contract: cryptographically check keys in ValidatorSe…

966fca6

…t in constructor

chore(ethereum): contract: more debuggable error on _modeExp()

1c8a438

feat(ethereum): contract: stop checking signatures when quorum is rea…

92140e9

…ched

Leonid Amirov added 2 commits March 5, 2026 15:41

chore(ethereum): formatting of Bridget.t.sol

6d1ddeb

feat(ethereum): contract tests: check sender test_ExecuteSignedIncrem…

5dbea27

…entsIdsAndEmitsEvent()

side2k enabled auto-merge March 5, 2026 12:50

side2k merged commit 9e94c47 into main Mar 5, 2026
4 checks passed

side2k deleted the ethereum-bridge-2-execute branch March 5, 2026 12:52

	vm.expectEmit(true, false, false, true, address(bridge));
	vm.expectEmit(true, true, false, true, address(bridge));

Conversation

side2k commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloudflare-workers-and-pages bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying kolme with Cloudflare Pages

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

side2k commented Mar 4, 2026 •

edited

Loading

cloudflare-workers-and-pages bot commented Mar 4, 2026 •

edited

Loading