Skip to content

relayer: support Google Cloud KMS signing - #584

Open
zmanian wants to merge 3 commits into
mainfrom
zaki/relayer-kms-signer
Open

relayer: support Google Cloud KMS signing#584
zmanian wants to merge 3 commits into
mainfrom
zaki/relayer-kms-signer

Conversation

@zmanian

@zmanian zmanian commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Why

The relayer accepted only a raw Ethereum private key via --ethereum-key=0x..., which is world-readable in /proc/<pid>/cmdline. On the production validator host that key was in fact exposed in the process table to every local account on the box.

SignerType already had a GcpKms variant and the gravity crate already depended on ethers-gcp-kms-signer, but nothing ever constructed a GcpKmsSigner — the KMS path was dead code. This wires it up.

Scope note: the relayer key pays gas to submit already-validator-signed batches. Relaying is permissionless, so this key does not authorize bridge transfers; exposure means gas-account drain and relaying disruption, not theft of bridge funds.

What

  • SignerType::new_gcp_kms() — wires GcpKeyRingRefGcpKmsProviderGcpKmsSigner.
  • New --gcp-kms-project, --gcp-kms-location, --gcp-kms-key-ring, --gcp-kms-key-name, --gcp-kms-key-version flags. Supply exactly one signing method; --ethereum-key still works and now warns about process-table exposure.
  • 30s timeout on all KMS signing calls. KMS signing is a network RPC with no deadline of its own, and the main loop drives valset, batch and logic-call relaying under a single tokio::join! — so a signing future that never resolved would have stopped all relaying rather than failing one submission. Timeouts surface as normal signer errors, which existing callers already log and retry.
  • --gcp-kms-key-version is required, not defaulted. A KMS signing address is version-specific: a silent default to 1 would fail to follow a rotation (relaying stops once v1 is disabled) and would hide that changing version changes the gas-paying Ethereum address.
  • Configuration errors return Result and exit cleanly instead of panicking.
  • Raw keys are parsed during argument handling, preserving the original fail-fast behaviour (constructing the KMS signer requires the chain ID, so signer construction moved below chain-ID resolution; key validation deliberately did not).
  • Debug is hand-written, not derived — the Local variant holds a private key and a derived impl would print it in full.

Testing

8 unit tests: signer selection, both-methods and no-method rejection, all 30 partial KMS flag combinations, eager key validation, and Debug redaction. cargo build and cargo clippy clean.

Not tested end to end: actual KMS signature production. GcpKmsSigner::new needs live GCP credentials, and the SignerType::normalize v-recovery path for KMS signatures has not been exercised against the real Gravity contract. Given main already carries #581 ("Normalize v in KMS signature"), that path warrants a testnet run before any mainnet cutover.

Operator notes

Each KMS key version has its own Ethereum address, which pays relayer gas and must be funded before switching. For sommelier-primary-eth-signer v1 that address is 0xe1a9190c225300468826736adec52fd2577a6e16, currently holding 0.001 ETH with nonce 0 — it has never sent a transaction, so it is not the account relaying today.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added support for signing Ethereum transactions with Google Cloud KMS.
    • Added command-line configuration for selecting either a local private key or Google Cloud KMS.
    • Added validation to prevent incomplete or conflicting signing configurations.
  • Bug Fixes

    • Added signing timeouts and clearer error handling for failed or unresponsive KMS operations.
    • Improved protection of sensitive local signing keys in diagnostic output.

zmanian and others added 3 commits August 5, 2026 08:53
The relayer accepted only a raw Ethereum private key via --ethereum-key,
which is visible in /proc/<pid>/cmdline to every local user on the host.
The SignerType abstraction already had a GcpKms variant, and the gravity
crate already depended on ethers-gcp-kms-signer, but nothing ever
constructed a GcpKmsSigner, so the KMS path was dead code.

Adds SignerType::new_gcp_kms(), which wires up GcpKeyRingRef ->
GcpKmsProvider -> GcpKmsSigner, and a SigningConfig in the relayer that
selects between the two. Passing --ethereum-key=kms selects KMS, which is
configured through GRAVITY_GCP_KMS_PROJECT, GRAVITY_GCP_KMS_LOCATION,
GRAVITY_GCP_KMS_KEY_RING and GRAVITY_GCP_KMS_KEY_NAME, with an optional
GRAVITY_GCP_KMS_KEY_VERSION defaulting to 1. Any other value is treated as
a raw key as before, and now logs a warning about process-table exposure.

Signer construction moves below chain-ID resolution because GcpKmsSigner
binds chain_id at construction. Signing configuration is resolved before
any network work so a misconfiguration fails immediately rather than after
connecting.

Note for operators: the KMS key derives its own Ethereum address, which
differs from the address of any raw key previously in use. That address
pays relayer gas and must be funded before switching.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bound KMS signing calls with a timeout. KMS signing is a network RPC and
the underlying client sets no deadline. The relayer main loop drives valset,
batch and logic-call relaying under a single tokio::join!, so a signing
future that never resolves would stop all relaying indefinitely rather than
failing one submission. All three signing paths now time out after 30s and
return a normal signer error, which existing callers already log and retry.

Require GRAVITY_GCP_KMS_KEY_VERSION rather than defaulting it to 1. A KMS
signing address is version-specific, so a silent default would neither
follow a key rotation, stopping relaying once version 1 is disabled, nor
make it evident that changing version changes the Ethereum address that
pays gas and so must be funded first.

Document the kms sentinel and its environment variables in --help, so the
mode is discoverable from the binary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses the remaining review recommendations.

Replace the --ethereum-key=kms sentinel with dedicated --gcp-kms-project,
--gcp-kms-location, --gcp-kms-key-ring, --gcp-kms-key-name and
--gcp-kms-key-version flags. The sentinel was an implicit type switch hidden
inside a key-valued option; it was originally chosen to avoid a docopt
limitation that turned out not to exist. Configuration now lives in flags
rather than environment variables, which also makes resolution a pure
function of Args and therefore testable.

Return Result from from_args and into_signer instead of panicking, so a
misconfiguration exits cleanly with an operator-facing message rather than a
backtrace.

Parse the raw --ethereum-key eagerly during argument handling. Moving signer
construction below chain-ID resolution for KMS had deferred key validation
until after connection setup, so a malformed key was no longer reported
immediately as it was before. SigningConfig::Local now holds a parsed wallet.

Implement Debug by hand rather than deriving it: the Local variant holds a
raw private key, and a derived Debug would print it in full anywhere the
value is formatted, which is the leak class this change exists to close.

Adds 8 unit tests covering signer selection, both-methods and no-method
rejection, all 30 partial KMS flag combinations, eager key validation, and
Debug redaction. End-to-end KMS signature production remains untested; it
needs live GCP credentials and a testnet run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The relayer now supports local Ethereum keys or GCP KMS signing. It validates signing options, constructs KMS signers after chain ID retrieval, and applies a 30-second timeout to KMS signing operations.

Changes

GCP KMS signing

Layer / File(s) Summary
KMS signer construction and timeout handling
orchestrator/gravity/src/ethereum/types.rs
Adds GCP KMS signer construction and applies timeout and error conversion to message, transaction, and typed-data signing.
Signing configuration validation
orchestrator/relayer/src/main.rs
Adds local-wallet and GCP KMS configuration variants, CLI options, validation, early key parsing, and debug redaction.
Chain-aware signer startup and validation
orchestrator/relayer/src/main.rs
Resolves configuration before network setup, retrieves the chain ID, constructs the selected signer, and tests valid and invalid configurations.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: hannydevelop, levicook

Sequence Diagram(s)

sequenceDiagram
  participant Relayer
  participant SigningConfig
  participant EthereumProvider
  participant SignerType
  participant GCPKMS

  Relayer->>SigningConfig: resolve and validate CLI options
  Relayer->>EthereumProvider: retrieve chain ID
  Relayer->>SignerType: construct local or GCP KMS signer
  SignerType->>GCPKMS: perform bounded signing operation
  GCPKMS-->>SignerType: return signature or error
  SignerType-->>Relayer: return signer result
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding Google Cloud KMS signing support to the relayer.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch zaki/relayer-kms-signer

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@orchestrator/gravity/src/ethereum/types.rs`:
- Around line 192-199: Update the typed-data recovery flow after
`payload.encode_eip712()` so the hash passed to `normalize` is an owned `[u8;
32]`/`H256` value rather than a borrowed byte slice. Preserve the existing GCP
KMS signature path while ensuring `sig.recover` treats the KMS-signed EIP-712
hash as raw data without adding the Ethereum signed-message prefix.

In `@orchestrator/relayer/src/main.rs`:
- Around line 272-278: Wrap the signing_config.into_signer(chain_id)
construction in the existing KMS_SIGN_TIMEOUT deadline, preserving the current
error logging and process exit behavior. Apply the timeout to the awaited future
so network calls made by GcpKmsProvider::new and GcpKmsSigner::new cannot hang
indefinitely, reusing the established timeout mechanism and symbol rather than
introducing a separate duration.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6ec0366b-787d-47d9-854c-f46e9854dfbe

📥 Commits

Reviewing files that changed from the base of the PR and between b63f7ca and 2f59299.

📒 Files selected for processing (2)
  • orchestrator/gravity/src/ethereum/types.rs
  • orchestrator/relayer/src/main.rs

Comment on lines +192 to +199
SignerType::GcpKms(signer) => {
with_kms_timeout("sign_typed_data", signer.sign_typed_data(payload)).await
}
}?;

// Get the typed data hash for recovery
let hash = payload.encode_eip712()
let hash = payload
.encode_eip712()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

ethers-rs Signature recover RecoveryMessage From impl slice hash prefix

💡 Result:

In ethers-rs, you can recover an Ethereum address from a signature using the Signature::recover method [1][2][3]. This method accepts any type that implements Into [2][4]. The RecoveryMessage enum is designed to handle two distinct scenarios [2][5]: 1. Data(Vec): If you provide raw message bytes, ethers-rs automatically handles the EIP-191 prefixing (i.e., hashing the message with the Ethereum Signed Message domain separator) before attempting recovery [2][4]. 2. Hash(H256): If you have already hashed your message (e.g., using Keccak256), you can pass the hash directly. In this case, the library treats it as a precomputed hash and skips the additional EIP-191 prefixing process [2][4]. To recover an address from a slice or hash, you can use the following patterns: // Recovering from raw message bytes (automatic EIP-191 hashing) let message = b"hello"; let recovered_address = signature.recover(&message[..])?; // Recovering from a precomputed hash (no extra prefixing) use ethers::core::types::H256; let hash = H256::from(keccak256("hello")); // Assuming custom hashing let recovered_address = signature.recover(hash)?; The Signature::recover implementation internally calls hash_message if you provide Data, or uses the raw hash if you provide Hash [2][4]. It then utilizes the VerifyingKey::recover_from_prehash method to derive the public key from the signature's recovery ID and the message hash [2][4]. Finally, it performs the standard Ethereum address derivation: taking the Keccak256 hash of the uncompressed public key (excluding the 0x04 prefix) and keeping the last 20 bytes [2][4].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate files =="
fd -a 'types\.rs$|cargo\.lock$|Cargo\.toml$' . | sed 's#^\./##' | head -100

echo
echo "== relevant context around mention/sign normalize =="
rg -n "normalize|recover|SignerType::GcpKms|encode_eip712|sign_typed_data|sign_transaction" -S .

echo
echo "== cargo deps ethers version =="
rg -n 'name = "ethers"|name = "ethers-core"|ethers =|ethers-core' Cargo.lock Cargo.toml -S || true

echo
echo "== git diff stat/name status =="
git diff --stat || true
git diff -- orchestrator/gravity/src/ethereum/types.rs | sed -n '1,220p' || true

Repository: PeggyJV/gravity-bridge

Length of output: 9700


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the Ethereum signer implementation and the normalize helpers without running repo code.
echo "== exact implementation slice =="
if [ -f orchestrator/gravity/src/ethereum/types.rs ]; then
  wc -l orchestrator/gravity/src/ethereum/types.rs
  sed -n '1,260p' orchestrator/gravity/src/ethereum/types.rs | nl -ba
fi

echo
echo "== dependency declarations with lock versions =="
if [ -f Cargo.lock ]; then
  awk '
    /^[[package]]\n/ {pkg=1}
    pkg && /^name = / {name = $0}
    pkg && /^version = / {version = $0}
    pkg && /^source = / {source = $0}
    pkg && /^ \/\// {next}
    /^[[package]]/ || /^[^[:space:]]/ || /^$/ {
      if (pkg && (name != "" || version != "")) {
        if (name ~ /ethers(-core)?/) print name version source
      }
      if (/^[[package]]/) { pkg=1; name=""; version=""; source="" }
      else if (/^$/) pkg=0;
    }
  ' Cargo.lock
elif [ -f Cargo.toml ]; then
  cat Cargo.toml | sed -n '1,220p'
fi

Repository: PeggyJV/gravity-bridge

Length of output: 280


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== orchestrator top-level files =="
find orchestrator -maxdepth 2 -type f | sed 's#^`#/`#' | sort | head -100

echo
echo "== relevant ethereum types.rs lines =="
python3 - <<'PY'
from pathlib import Path
p = Path("orchestrator/gravity/src/ethereum/types.rs")
for i, line in enumerate(p.read_text().splitlines(), 1):
    if 1 <= i <= 210:
        print(f"{i:4}\t{line}")
PY

echo
echo "== dependency/version declarations =="
python3 - <<'PY'
from pathlib import Path
for p in [Path("Cargo.lock"), Path("Cargo.toml"), Path("orchestrator/Cargo.lock"), Path("orchestrator/Cargo.toml"), Path("orchestrator/gravity/Cargo.toml")]:
    if p.exists():
        print(f"\n-- {p} --")
        text = p.read_text()
        for line in text.splitlines():
            if "ethers" in line.lower() or line.startswith("[") and "version" in line:
                print(line)
PY

echo
echo "== local cargo lock crates around ethers =="
python3 - <<'PY' from pathlib import Path
for root in [Path("orchestrator"), Path(".")]:
    lock = root / "Cargo.lock"
    if lock.exists():
        print(f"-- {lock} --")
        lines = lock.read_text().splitlines()
        for i,l in enumerate(lines):
            if 'name = "ethers"' in l or 'name = "ethers-core"' in l or 'name = "ethers-contract"' in l or 'name = "k256"' in l:
                print("\n".join(lines[max(0,i-4):i+6]))
PY

echo
echo "== signature recovery usage in orchestrator tests/utils =="
python3 - <<'PY'
from pathlib import Path
for p in Path("orchestrator").rglob("*.rs"):
    s = p.read_text().splitlines()
    hit=False
    for i,l in enumerate(s,1):
        if "recover" in l and "signature" in p.name:
            hit=True
            start=max(1,i-4); end=min(len(s),i+8)
            print(f"\n-- {p}:{start}-{end} --")
            for n in range(start,end+1):
                print(f"{n:4}\t{s[n-1]}")
PY

Repository: PeggyJV/gravity-bridge

Length of output: 11039


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Cargo.lock relevant crates =="
python3 - <<'PY'
from pathlib import Path
p=Path("orchestrator/Cargo.lock")
lines=p.read_text().splitlines()
for i,l in enumerate(lines):
    if 'name = "ethers"' in l or 'name = "ethers-core"' in l or 'name = "k256"' in l:
        print("\n".join(lines[max(0,i-4):i+6]))
        print()
PY

echo
echo "== check available curl/jq =="
command -v curl || true
command -v jq || true

echo
echo "== fetch ethers v2.0.7 signature implementation from crates.io =="
tmpdir="$(mktemp -d)"
url="https://crates.io/api/v1/crates/ethers-core/2.0.7/download"
status="$(curl -LfsSw '%{http_code}' "$url" -o "$tmpdir/ethers-core.tar.gz")"
echo "fetch_status=$status"
if [ "$status" = "200" ]; then
  tar -xzf "$tmpdir/ethers-core.tar.gz" -C "$tmpdir"
  found="$(find "$tmpdir" -path '*src/types/signature.rs' | head -1)"
  echo "signature_file=$found"
  if [ -n "$found" ]; then
    rg -n "impl From<|enum RecoveryMessage|recover_message|RecoveryMessage::|hash_message|Signature::recover|impl AsRef" "$found" -C 3
    echo
    sed -n '1,220p' "$found"
  fi
else
  echo "could not fetch ethers-core 2.0.7"
fi

Repository: PeggyJV/gravity-bridge

Length of output: 1326


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== actual dependency versions =="
python3 - <<'PY'
from pathlib import Path
p=Path("orchestrator/Cargo.lock")
lines=p.read_text().splitlines()
for i,l in enumerate(lines):
    if ('name = "ethers"' in l or 'name = "ethers-core"' in l) and i+1 < len(lines) and 'version =' in lines[i+1]:
        print(lines[i], lines[i+1])
PY

echo
echo "== fetch ethers-core v2.0.14 signature implementation =="
tmpdir="$(mktemp -d)"
pkg_url="https://crates.io/api/v1/crates/ethers-core/2.0.14/download"
json="$(curl -Lfs "$pkg_url")"
status="$(echo "$json" | jq -r '.version.downloads // empty')"
echo "json_version=$(echo "$json" | jq -r '.version.num // empty')"
echo "downloader=$status"
if [ -n "$status" ]; then
  rm -f "$tmpdir/ethers-core-*"
  echo "$json" | jq -r '.version.tarball' | xargs -r -I{} curl -LfsS {} -o "$tmpdir/ethers-core.tar.gz"
  tar -xzf "$tmpdir/ethers-core.tar.gz" -C "$tmpdir"
  found="$(find "$tmpdir" -path '*src/types/signature.rs' | head -1)"
  echo "signature_file=$found"
  rg -n "impl From<|enum RecoveryMessage|recover_message|RecoveryMessage::|hash_message|Signature::recover|impl AsRef" "$found" -C 4 || true
  echo
  sed -n '1,230p' "$found"
fi

Repository: PeggyJV/gravity-bridge

Length of output: 328


🌐 Web query:

crates.io ethers-core 2.0.14 source signature.rs ethers_core::types::signature

💡 Result:

In the ethers-core crate version 2.0.14, the Signature struct is defined in the source file src/types/signature.rs [1][2]. This struct represents an ECDSA signature and contains the following fields: [3][4] pub struct Signature { pub r: U256, pub s: U256, pub v: u64, } The Signature implementation includes methods for verifying messages and recovering addresses, such as recover, recover_typed_data, and verify [3][4]. These methods typically use Electrum notation for the v value (expected to be 27 or 28) [4]. The source code for this specific version can be accessed directly through the Docs.rs platform [1][2]. Note that the ethers-rs library is in the process of being deprecated, and users are encouraged to look toward newer alternatives [5][6].

Citations:


🌐 Web query:

docs.rs 2.0.14 ethers_core::types::signature Signature recover RecoveryMessage impl From<[u8; 32]>

💡 Result:

In ethers-core 2.0.14, the RecoveryMessage enum is used to represent the message data that can be signed or verified [1][2]. It provides an implementation of the From trait for [u8; 32], which allows a 32-byte array (typically a message hash) to be converted directly into a RecoveryMessage variant [2][3]. Specifically, the implementation is defined as follows: impl From<[u8; 32]> for RecoveryMessage { fn from(hash: [u8; 32]) -> Self { RecoveryMessage::Hash(hash.into) } } When used with the Signature::recover method, which accepts any type M that implements Into, passing a [u8; 32] will treat the data as a precomputed message hash [1][2]. If the input is instead RecoveryMessage::Data(Vec), the library will automatically hash the message using EIP-191 before performing the recovery [1][2].

Citations:


🌐 Web query:

docs.rs ethers-core 2.0.14 source impl From<&[u8]> RecoveryMessage Data prefix

💡 Result:

In ethers-core version 2.0.14, the RecoveryMessage enum is defined in src/types/signature.rs [1][2]. The implementation of From<&[u8]> for RecoveryMessage converts a slice of bytes into a RecoveryMessage::Data variant by converting the slice into a Vec and then into the enum [1][2]. Regarding the specific mention of a data prefix, the RecoveryMessage enum itself does not contain a prefixing implementation within its From conversion [1][2]. Instead, the prefixing logic for Ethereum signed messages is handled by the hash_message function, located in src/utils/hash.rs [3]. This function implements EIP-191 by prepending the prefix "\x19Ethereum Signed Message:\n" and the decimal length of the message to the input bytes before hashing them with Keccak-256 [3]. The source code for the relevant implementation is as follows: // From src/types/signature.rs in ethers-core 2.0.14 [2] impl From<&[u8]> for RecoveryMessage { fn from(s: &[u8]) -> Self { s.to_owned.into } } // From src/utils/hash.rs in ethers-core 2.0.14 [3] pub fn hash_message<T: AsRef<[u8]>>(message: T) -> H256 { const PREFIX: &str = "\x19Ethereum Signed Message:\n"; let message = message.as_ref; let len = message.len; let len_string = len.to_string; let mut eth_message = Vec::with_capacity(PREFIX.len + len_string.len + len); eth_message.extend_from_slice(PREFIX.as_bytes); eth_message.extend_from_slice(len_string.as_bytes); eth_message.extend_from_slice(message); H256(keccak256(&eth_message)) }

Citations:


🌐 Web query:

docs.rs ethers-core 2.0.14 source Signature recover MessageSignature signature.rs line

💡 Result:

In version 2.0.14 of the ethers-core crate, the recover method for the Signature struct is defined within the src/types/signature.rs file [1][2]. The implementation of the recover method, which recovers the Ethereum address used to sign a given message, typically begins around line 91 of the source file [2]. The method accepts a message that implements the Into trait and returns a Result containing an Address or a SignatureError [1][2]. Within this method, the signature is converted into a recoverable signature and a recovery ID using the internal as_signature method [2]. It then utilizes the VerifyingKey::recover_from_prehash function from the underlying elliptic curve library to recover the public key, which is subsequently hashed to derive the Ethereum address [2]. Note that ethers-core is part of a library that is in the process of being deprecated, and it is recommended to use the re-exports directly from the ethers crate instead [3][4].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check whether crates.io supports a stable source tarball URL for ethers-core 2.0.14.
pkg="ethers-core"
ver="2.0.14"
crates_api="$(curl -LfsS "https://crates.io/api/v1/crates/$pkg/$ver/download" | jq -r '.download_url // empty' 2>/dev/null || true)"
echo "api_download_url=$crates_api"
if [ -n "$crates_api" ]; then
  tmpdir="$(mktemp -d)"
  curl -LfsS "$crates_api" -o "$tmpdir/ethers-core.tar.gz"
  tar -xzf "$tmpdir/ethers-core.tar.gz" -C "$tmpdir"
  found="$(find "$tmpdir" -path '*src/types/signature.rs' | head -1)"
  echo "signature_file=$found"
  rg -n "impl From<|enum RecoveryMessage|recover_message|RecoveryMessage::|hash_message|Signature::recover|impl AsRef" "$found" -C 3 || true
  echo "--- signature.rs top ---"
  sed -n '1,180p' "$found"
fi

echo
echo "== available GitHub API crate source refs =="
python3 - <<'PY'
import urllib.request
url="https://crates.io/api/v1/crates/ethers-core/2.0.14"
text=urllib.request.urlopen(url, timeout=20).read().decode()
print(text[:2000])
PY

Repository: PeggyJV/gravity-bridge

Length of output: 2559


Pass an owned hash into the GCP KMS recovery path.

normalize(&hash, &sig) passes a &[u8] into sig.recover(...), and ethers treats &[u8] as raw data that receives the Ethereum signed-message prefix. KMS signs the raw EIP-712 hash, so recovery for typed-data and transaction signatures can return “Invalid signature while normalizing”. Copy the [u8; 32]/H256 value before calling normalize so it is treated as a raw hash.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@orchestrator/gravity/src/ethereum/types.rs` around lines 192 - 199, Update
the typed-data recovery flow after `payload.encode_eip712()` so the hash passed
to `normalize` is an owned `[u8; 32]`/`H256` value rather than a borrowed byte
slice. Preserve the existing GCP KMS signature path while ensuring `sig.recover`
treats the KMS-signed EIP-712 hash as raw data without adding the Ethereum
signed-message prefix.

Comment on lines +272 to +278
let ethereum_wallet = signing_config
.into_signer(chain_id)
.await
.unwrap_or_else(|e| {
error!("{}", e);
std::process::exit(1);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Bound the KMS signer construction with a timeout.

into_signer performs network calls for the KMS variant. GcpKmsProvider::new and GcpKmsSigner::new contact the credential source and the KMS API to fetch the public key. Neither call has a deadline here. If the metadata server or the KMS endpoint does not respond, the relayer hangs at startup after the chain-ID step and logs nothing further.

The signing paths already apply KMS_SIGN_TIMEOUT for this exact reason. Apply the same bound at construction.

🛡️ Proposed fix to bound signer construction
     // GcpKmsSigner binds chain_id at construction, so the signer can only be
     // built once the chain ID is known.
-    let ethereum_wallet = signing_config
-        .into_signer(chain_id)
-        .await
-        .unwrap_or_else(|e| {
-            error!("{}", e);
-            std::process::exit(1);
-        });
+    let ethereum_wallet = match tokio::time::timeout(
+        std::time::Duration::from_secs(30),
+        signing_config.into_signer(chain_id),
+    )
+    .await
+    {
+        Ok(Ok(signer)) => signer,
+        Ok(Err(e)) => {
+            error!("{}", e);
+            std::process::exit(1);
+        }
+        Err(_) => {
+            error!("Timed out constructing the Ethereum signer");
+            std::process::exit(1);
+        }
+    };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let ethereum_wallet = signing_config
.into_signer(chain_id)
.await
.unwrap_or_else(|e| {
error!("{}", e);
std::process::exit(1);
});
let ethereum_wallet = match tokio::time::timeout(
std::time::Duration::from_secs(30),
signing_config.into_signer(chain_id),
)
.await
{
Ok(Ok(signer)) => signer,
Ok(Err(e)) => {
error!("{}", e);
std::process::exit(1);
}
Err(_) => {
error!("Timed out constructing the Ethereum signer");
std::process::exit(1);
}
};
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@orchestrator/relayer/src/main.rs` around lines 272 - 278, Wrap the
signing_config.into_signer(chain_id) construction in the existing
KMS_SIGN_TIMEOUT deadline, preserving the current error logging and process exit
behavior. Apply the timeout to the awaited future so network calls made by
GcpKmsProvider::new and GcpKmsSigner::new cannot hang indefinitely, reusing the
established timeout mechanism and symbol rather than introducing a separate
duration.

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.

1 participant