fix domain bug, harden implementation

Author: GCdePaula

examples/app-core/src/application/wallet.rs

@@ -145,14 +145,8 @@ impl Application for WalletApp {
             });
         }
 
-        let max_fee = user_op.max_fee;
-        // Users sign a cap (log-space exponent); sequencer executes against the committed frame fee.
-        if max_fee < current_fee {
-            return Err(InvalidReason::InvalidMaxFee {
-                max_fee,
-                base_fee: current_fee,
-            });
-        }
+        // max_fee < current_fee is already checked by the trait default in
+        // validate_and_execute_user_op. No need to repeat here.
 
         let gas_cost = sequencer_core::fee::fee_to_linear(current_fee);
         let balance = self.balance_of(&sender);
@@ -279,6 +273,8 @@ mod tests {
 
     #[test]
     fn validate_rejects_when_max_fee_below_current_fee() {
+        use sequencer_core::application::{Application, ExecutionOutcome};
+
         let mut app = WalletApp::new(WalletConfig::default());
         let sender = Address::from_slice(&[0x11; 20]);
         app.balances.insert(sender, U256::from(10_u64));
@@ -289,15 +285,17 @@ mod tests {
             data: Vec::<u8>::new().into(),
         };
 
-        let err = app
-            .validate_user_op(sender, &user_op, 2)
-            .expect_err("max_fee < current_fee should be invalid");
+        // The max_fee < current_fee check now lives in the trait default
+        // (validate_and_execute_user_op), not in validate_user_op directly.
+        let result = app
+            .validate_and_execute_user_op(sender, &user_op, 2)
+            .expect("should return Ok(Invalid), not Err");
         assert_eq!(
-            err,
-            InvalidReason::InvalidMaxFee {
+            result,
+            ExecutionOutcome::Invalid(InvalidReason::InvalidMaxFee {
                 max_fee: 1,
                 base_fee: 2
-            }
+            })
         );
     }
 

examples/canonical-app/src/scheduler/core.rs

@@ -244,6 +244,8 @@ impl<A: Application> Scheduler<A> {
         for user_op in &frame.user_ops {
             if let Some(sender) = self.recover_sender(domain, user_op) {
                 let plain = user_op.to_user_op();
+                // Defense-in-depth: the trait default in validate_and_execute_user_op
+                // now centralizes this check, but we keep it here as an extra guard.
                 if plain.max_fee < frame.fee_price {
                     eprintln!("scheduler skipped frame user-op due to max_fee < fee_price");
                     continue;
@@ -326,13 +328,7 @@ fn has_elapsed_since(start_block: u64, wait_blocks: u64, current_block: u64) ->
 }
 
 pub(super) fn input_domain(chain_id: u64, verifying_contract: Address) -> Eip712Domain {
-    Eip712Domain {
-        name: None,
-        version: None,
-        chain_id: Some(U256::from(chain_id)),
-        verifying_contract: Some(verifying_contract),
-        salt: None,
-    }
+    sequencer_core::build_input_domain(chain_id, verifying_contract)
 }
 
 pub(super) fn block_to_u64(block: U256) -> u64 {

examples/canonical-test/src/main.rs

@@ -231,13 +231,7 @@ fn devnet_machine() -> Result<Machine, Box<dyn std::error::Error + Send + Sync>>
 }
 
 fn input_domain() -> Eip712Domain {
-    Eip712Domain {
-        name: None,
-        version: None,
-        chain_id: Some(U256::from(TEST_CHAIN_ID)),
-        verifying_contract: Some(TEST_DAPP_ADDRESS),
-        salt: None,
-    }
+    sequencer_core::build_input_domain(TEST_CHAIN_ID, TEST_DAPP_ADDRESS)
 }
 
 fn signing_key(byte: u8) -> SigningKey {

sequencer-core/src/application/mod.rs

@@ -102,6 +102,15 @@ pub trait Application: Send {
         user_op: &UserOp,
         current_fee: u16,
     ) -> Result<ExecutionOutcome, AppError> {
+        // Protocol invariant: max_fee must cover the current frame fee.
+        // Enforced here so every Application impl inherits it.
+        if user_op.max_fee < current_fee {
+            return Ok(ExecutionOutcome::Invalid(InvalidReason::InvalidMaxFee {
+                max_fee: user_op.max_fee,
+                base_fee: current_fee,
+            }));
+        }
+
         if let Err(reason) = self.validate_user_op(sender, user_op, current_fee) {
             return Ok(ExecutionOutcome::Invalid(reason));
         }

sequencer-core/src/batch.rs

@@ -4,10 +4,6 @@
 use crate::user_op::UserOp;
 use ssz_derive::{Decode, Encode};
 
-/// Tag byte for InputBox payloads that are L1 app direct inputs (e.g. deposits).
-/// L1/app must post such inputs as `0x00 || body`. Only these are stored (body only) and executed.
-pub const INPUT_TAG_DIRECT_INPUT: u8 = 0x00;
-
 // ---------------------------------------------------------------------------
 // Gas-economics-derived batch sizing
 //

sequencer-core/src/lib.rs

@@ -1,6 +1,9 @@
 // (c) Cartesi and individual authors (see AUTHORS)
 // SPDX-License-Identifier: Apache-2.0 (see LICENSE)
 
+use alloy_primitives::{Address, U256};
+use alloy_sol_types::Eip712Domain;
+
 pub mod api;
 pub mod application;
 pub mod batch;
@@ -12,3 +15,25 @@ pub mod user_op;
 /// Maximum number of L1 blocks a batch can wait before the scheduler considers it stale.
 /// Shared between the scheduler (canonical-app) and the sequencer (batch submitter, startup detection).
 pub const MAX_WAIT_BLOCKS: u64 = 1200;
+
+/// EIP-712 domain name shared between sequencer and scheduler.
+pub const DOMAIN_NAME: &str = "CartesiAppSequencer";
+
+/// EIP-712 domain version shared between sequencer and scheduler.
+pub const DOMAIN_VERSION: &str = "1";
+
+/// Build the canonical EIP-712 domain for user-op signing and verification.
+///
+/// Both the sequencer (signature verification at ingress) and the scheduler
+/// (signature recovery during batch execution) MUST use this constructor.
+/// A mismatch in any field changes the domain separator and causes every
+/// signature to recover a different address.
+pub fn build_input_domain(chain_id: u64, verifying_contract: Address) -> Eip712Domain {
+    Eip712Domain {
+        name: Some(DOMAIN_NAME.into()),
+        version: Some(DOMAIN_VERSION.into()),
+        chain_id: Some(U256::from(chain_id)),
+        verifying_contract: Some(verifying_contract),
+        salt: None,
+    }
+}

sequencer/src/ingress/api.rs

@@ -89,13 +89,23 @@ async fn submit_tx(
     }))
 }
 
-/// Normalize JSON-extractor failures: 413 stays 413, everything else becomes
-/// 400. Keeps the public API contract stable across axum upgrades.
+/// Normalize JSON-extractor failures into fixed client-facing messages.
+/// Keeps the public API contract stable across axum upgrades and avoids
+/// reflecting parser internals (serde line/column, token excerpts) to callers.
 fn map_json_rejection(err: axum::extract::rejection::JsonRejection) -> ApiError {
+    use axum::extract::rejection::JsonRejection;
+
+    tracing::debug!(error = %err, "JSON extraction failed");
+
     if err.status() == StatusCode::PAYLOAD_TOO_LARGE {
-        ApiError::payload_too_large(format!("request body too large: {err}"))
+        ApiError::payload_too_large("request body too large")
     } else {
-        ApiError::bad_request(format!("invalid JSON: {err}"))
+        match err {
+            JsonRejection::MissingJsonContentType(_) => {
+                ApiError::bad_request("missing content type")
+            }
+            _ => ApiError::bad_request("invalid JSON"),
+        }
     }
 }
 

sequencer/src/ingress/inclusion_lane/mod.rs

@@ -242,7 +242,7 @@ impl<A: Application + 'static> InclusionLane<A> {
         self.storage
             .append_user_ops_chunk(head, included.as_slice())
             .map_err(|err| {
-                Self::respond_internal_to_all(included, format!("db error: {err}"));
+                Self::respond_internal_to_all(included, "internal storage error".to_string());
                 InclusionLaneError::Storage(err)
             })
     }

sequencer/src/ingress/inclusion_lane/tests.rs

@@ -241,7 +241,9 @@ fn make_pending_user_op(
     let (respond_to, recv) = oneshot::channel();
     let user_op = UserOp {
         nonce: 0,
-        max_fee: 1,
+        // Must be >= the DB default recommended_fee (1060) to pass the
+        // protocol-level max_fee >= fee_price check in the trait default.
+        max_fee: u16::MAX,
         data: vec![seed; 4].into(),
     };
     (

sequencer/src/l1/provider.rs

@@ -20,6 +20,15 @@ const COMPUTE_UNITS_PER_SEC: u64 = 500;
 fn create_client(url: &str) -> Result<RpcClient, String> {
     let url = Url::parse(url).map_err(|e| format!("invalid RPC URL: {e}"))?;
 
+    // Reject non-HTTPS for remote hosts to prevent accidental plaintext RPC.
+    let host = url.host_str().unwrap_or("");
+    if url.scheme() != "https" && !matches!(host, "localhost" | "127.0.0.1" | "::1") {
+        return Err(format!(
+            "remote RPC must use https, got {}://",
+            url.scheme()
+        ));
+    }
+
     let http_client = reqwest::Client::builder()
         .timeout(REQUEST_TIMEOUT)
         .build()
@@ -50,7 +59,7 @@ pub fn create_provider(url: &str) -> Result<DynProvider, String> {
 pub fn create_signer_provider(url: &str, private_key: &str) -> Result<DynProvider, String> {
     let client = create_client(url)?;
     let signer =
-        PrivateKeySigner::from_str(private_key).map_err(|e| format!("invalid private key: {e}"))?;
+        PrivateKeySigner::from_str(private_key).map_err(|_| "invalid private key".to_string())?;
     let provider = ProviderBuilder::new().wallet(signer).connect_client(client);
     Ok(provider.erased())
 }