[Feat] change sign tx evm logic by tanut32039 · Pull Request #24 · bandprotocol/fkms

tanut32039 · 2026-04-20T04:44:12Z

let falcon send tx field to fkms and let it create and sign tx

Copilot

Pull request overview

This PR shifts EVM signing from “Falcon sends a full encoded tx to FKMS” to “Falcon sends structured tx fields + TSS data, and FKMS builds the tx, computes the signing hash, signs it, and returns a fully signed EIP-2718 tx blob”.

Changes:

Replace EVM tx decoding + Keccak256(message) signing with alloy-based tx construction, signing-hash computation, and signed-tx encoding.
Update the gRPC/proto API for EVM signing to use EvmSignerPayload + Tss, and return tx_blob instead of a raw signature.
Add alloy crates needed for transaction hashing/encoding.

Reviewed changes

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

Show a summary per file

File	Description
src/server/service.rs	Builds `EvmTxParams` from proto payload, computes signing hash, signs, and returns a full signed tx blob.
src/codec/evm.rs	Introduces calldata builder, tx param struct, signing-hash computation, and signed-tx encoding via alloy.
proto/fkms/v1/signer.proto	Changes `SignEvmRequest/Response` to structured payload + returns `tx_blob`.
Cargo.toml	Adds `alloy-consensus` and `alloy-eips` dependencies.
Cargo.lock	Locks new dependency graph versions.

Comments suppressed due to low confidence (1)

src/codec/evm.rs:10

The sol! { struct Tss { ... } } type is no longer referenced in this module after removing decode_tx, which will trigger an unused-item warning. Remove it (or use it) to keep the module minimal and avoid warning-as-error CI failures.

sol! {
    struct Tss {
        bytes message;
        address randomAddr;
        uint256 signature;

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

Copilot · 2026-04-21T07:54:18Z

+    let gas_price = if !p.gas_price.is_empty() {
+        Some(U256::from_be_slice(&p.gas_price).to::<u128>())
+    } else {
+        None


U256::from_be_slice(...).to::<u128>() truncates on overflow, so oversized gas values can silently wrap and produce a different transaction than the caller intended. Use a checked conversion (error if > u128::MAX) or store gas fields as U256 end-to-end. This applies to gas_price, gas_fee_cap, and gas_tip_cap.

Copilot · 2026-04-21T07:54:19Z

+        let tss = sign_evm_request
+            .tss
+            .ok_or_else(|| Status::invalid_argument("tss field is required and cannot be null"))?;



sign_evm currently does not verify the TSS signature via self.tss_signature_verifier, while the other sign_* endpoints do. Since tss.message/random_addr/signature_s are used to build calldata and ultimately the signed transaction, add the same verification block here (return invalid_argument on failure) to avoid signing unverified TSS payloads.

Suggested change

self.tss_signature_verifier

.verify(&tss.message, &tss.random_addr, &tss.signature_s)

.map_err(|_| Status::invalid_argument("Invalid TSS signature"))?;

Copilot · 2026-04-21T07:54:19Z

+                let params = build_evm_tx_params(&signer_payload, &tss)
+                    .map_err(|e| Status::internal(format!("Failed to build tx params: {e}")))?;


Errors from build_evm_tx_params that are caused by client input (e.g., invalid to address or malformed gas fields) are currently mapped to Status::internal. Consider mapping validation/parsing failures to Status::invalid_argument so callers get actionable 4xx responses.

Suggested change

let params = build_evm_tx_params(&signer_payload, &tss)

.map_err(|e| Status::internal(format!("Failed to build tx params: {e}")))?;

let params = build_evm_tx_params(&signer_payload, &tss).map_err(|e| {

Status::invalid_argument(format!("Failed to build tx params: {e}"))

})?;

Copilot · 2026-04-21T07:54:19Z

+  bytes  gas_price  = 6;  // big-endian big.Int bytes
+  // EIP-1559 fields
+  bytes  gas_fee_cap = 7; // big-endian big.Int bytes (maxFeePerGas)
+  bytes  gas_tip_cap = 8; // big-endian big.Int bytes (maxPriorityFeePerGas)


The gas fields are documented as “big-endian big.Int bytes”, but the server code decodes them into u128 (and should error on overflow). Either document the effective max size (<= u128) or update the API/implementation to support full 256-bit gas values to match the comment.

Suggested change

bytes gas_price = 6; // big-endian big.Int bytes

// EIP-1559 fields

bytes gas_fee_cap = 7; // big-endian big.Int bytes (maxFeePerGas)

bytes gas_tip_cap = 8; // big-endian big.Int bytes (maxPriorityFeePerGas)

bytes gas_price = 6; // big-endian integer bytes; value must be non-negative and fit in u128

// EIP-1559 fields

bytes gas_fee_cap = 7; // big-endian integer bytes; value must be non-negative and fit in u128 (maxFeePerGas)

bytes gas_tip_cap = 8; // big-endian integer bytes; value must be non-negative and fit in u128 (maxPriorityFeePerGas)

Copilot · 2026-04-21T07:54:20Z

+/// Computes the signing hash for the transaction (Keccak256 of the signing payload).
+pub fn compute_signing_hash(params: &EvmTxParams) -> anyhow::Result<[u8; 32]> {
+    match (params.gas_price, params.gas_fee_cap, params.gas_tip_cap) {
+        (Some(gas_price), None, None) => {
+            let tx = build_legacy_tx(params, gas_price);


EVM signing behavior changed substantially (signing-hash computation + signed-tx encoding) but the previous EVM unit tests were removed and no new tests were added. Add tests covering legacy vs EIP-1559 signing hashes and tx encoding (including v/parity handling) to prevent serialization regressions.

Copilot · 2026-04-21T07:54:20Z

+) -> anyhow::Result<Vec<u8>> {
+    let call = relayCall {
+        message: message.to_vec().into(),
+        randomAddr: Address::from_slice(random_addr),
+        signature: U256::from_be_slice(signature_s),


Address::from_slice(random_addr) will panic if random_addr is not exactly 20 bytes, which can crash the server on malformed/untrusted input. Validate the length (and return an error) before constructing the Address (or use a fallible conversion) so this function never panics.

Copilot · 2026-04-21T07:54:20Z

+    // parity is the recovery bit: non-zero means true (odd y)
+    let sig = Signature::from_bytes_and_parity(&sig_bytes[..64], sig_bytes[64] != 0);



sig_bytes[64] != 0 is not a correct y-parity derivation for common ECDSA recovery-id encodings (e.g., 2/3 or 27/28). This will produce invalid signed tx blobs for some valid signatures. Normalize v (e.g., map 27/28 to 0/1 and then use v % 2), and reject unexpected values.

Suggested change

// parity is the recovery bit: non-zero means true (odd y)

let sig = Signature::from_bytes_and_parity(&sig_bytes[..64], sig_bytes[64] != 0);

// Normalize common Ethereum recovery-id encodings to parity:

// 0/1 => 0/1, 27/28 => 0/1, 2/3 => 0/1.

let normalized_v = match sig_bytes[64] {

0 | 1 => sig_bytes[64],

27 | 28 => sig_bytes[64] - 27,

2 | 3 => sig_bytes[64] % 2,

v => {

return Err(anyhow::anyhow!(

"unsupported signature recovery id: {v}; expected 0/1, 2/3, or 27/28"

));

}

};

let sig = Signature::from_bytes_and_parity(&sig_bytes[..64], normalized_v != 0);

add sign tx evm logic

871526d

tanut32039 requested review from Marca23 and RogerKSI April 20, 2026 04:44

remove unused import

351ea2e

RogerKSI requested a review from Copilot April 21, 2026 07:48

Copilot started reviewing on behalf of RogerKSI April 21, 2026 07:48 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat] change sign tx evm logic#24

[Feat] change sign tx evm logic#24
tanut32039 wants to merge 2 commits intomainfrom
change-evm-tx-signing

tanut32039 commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

+        self.tss_signature_verifier
+            .verify(&tss.message, &tss.random_addr, &tss.signature_s)
+            .map_err(|_| Status::invalid_argument("Invalid TSS signature"))?;

		let params = build_evm_tx_params(&signer_payload, &tss)
		.map_err(\|e\| Status::internal(format!("Failed to build tx params: {e}")))?;

		// parity is the recovery bit: non-zero means true (odd y)
		let sig = Signature::from_bytes_and_parity(&sig_bytes[..64], sig_bytes[64] != 0);

-    // parity is the recovery bit: non-zero means true (odd y)
-    let sig = Signature::from_bytes_and_parity(&sig_bytes[..64], sig_bytes[64] != 0);
+    // Normalize common Ethereum recovery-id encodings to parity:
+    // 0/1 => 0/1, 27/28 => 0/1, 2/3 => 0/1.
+    let normalized_v = match sig_bytes[64] {
+| 1 => sig_bytes[64],
+| 28 => sig_bytes[64] - 27,
+| 3 => sig_bytes[64] % 2,
+        v => {
+            return Err(anyhow::anyhow!(
+                "unsupported signature recovery id: {v}; expected 0/1, 2/3, or 27/28"
+            ));
+        }
+    };
+    let sig = Signature::from_bytes_and_parity(&sig_bytes[..64], normalized_v != 0);

Conversation

tanut32039 commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants