Skip to content

feat: enhance gasless key management with routing and validation improvements#794

Merged
topocount merged 3 commits intomainfrom
kjs/neyn-10580-shard-0-routing-for-key-messages
Apr 24, 2026
Merged

feat: enhance gasless key management with routing and validation improvements#794
topocount merged 3 commits intomainfrom
kjs/neyn-10580-shard-0-routing-for-key-messages

Conversation

@topocount
Copy link
Copy Markdown
Contributor

  • Updated message routing to direct KEY_ADD and KEY_REMOVE to shard 0 since they are stateful cros-fid interactions
  • Implemented conversion of ShardEngine validation errors to BlockEngine variants
  • Added merge functions for KEY_ADD and KEY_REMOVE to streamline processing across both engines.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
snapchain-docs Ready Ready Preview, Comment Apr 24, 2026 0:10am

Request Review

…ovements

- Updated message routing to direct KEY_ADD and KEY_REMOVE to shard 0 since they are stateful cros-fid interactions
- Implemented conversion of ShardEngine validation errors to BlockEngine variants
- Added merge functions for KEY_ADD and KEY_REMOVE to streamline processing across both engines.
@topocount topocount force-pushed the kjs/neyn-10580-shard-0-routing-for-key-messages branch from c202f6d to a6aeb60 Compare April 24, 2026 12:09
@github-actions
Copy link
Copy Markdown

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • src/mempool/routing.rs (96.9%): Missing lines 104
  • src/storage/store/account/gasless_key_merge.rs (0.0%): Missing lines 51-63,65-67,74-75,81-87,89-95,97-99,103-120,122-124,130-138,143,148-159,161-168,203-215,217-219,222,226-227,234-235,237-251,253-256,263-266,273-274,276-283
  • src/storage/store/block_engine.rs (75.6%): Missing lines 286,323,341-352,448-450,1084,1099,1103
  • src/storage/store/engine.rs (72.1%): Missing lines 1227-1238

Summary

  • Total: 297 lines
  • Missing: 173 lines
  • Coverage: 41%

src/mempool/routing.rs

  100         let router: Box<dyn MessageRouter> = Box::new(ShardRouter {});
  101         let shard = route_message(&router, &msg(MessageType::CastAdd, 12345), 2);
  102         assert!(
  103             shard == 1 || shard == 2,
! 104             "expected shard 1 or 2, got {shard}"
  105         );
  106     }
  107 }

src/storage/store/account/gasless_key_merge.rs

  47 ///
  48 /// Active-key cap (1000 per FID combined) and pending-FID resolution via
  49 /// `registration_tx_hash` are intentionally out of scope — they're owned by NEYN-10579 and
  50 /// NEYN-10577 respectively and block this ticket only at the future-merge level.
! 51 pub fn merge_key_add(
! 52     db: &Arc<RocksDB>,
! 53     onchain_event_store: &OnchainEventStore,
! 54     msg: &proto::Message,
! 55     txn_batch: &mut RocksDbTransactionBatch,
! 56 ) -> Result<HubEvent, MessageValidationError> {
! 57     let message_data = msg
! 58         .data
! 59         .as_ref()
! 60         .ok_or(MessageValidationError::NoMessageData)?;
! 61     let fid = message_data.fid;
! 62     let key_add_body = match &message_data.body {
! 63         Some(Body::KeyAddBody(body)) => body,
  64         _ => {
! 65             return Err(MessageValidationError::InvalidMessageType(
! 66                 message_data.r#type,
! 67             ))
  68         }
  69     };
  70 
  71     // Use the message's own timestamp as the reference for deadline checks. This is

  70 
  71     // Use the message's own timestamp as the reference for deadline checks. This is
  72     // deterministic across nodes (replay-safe) and upstream `validate_message` has already
  73     // bounded `message.timestamp` to a reasonable window around current block time.
! 74     let current_timestamp = message_data.timestamp as u64;
! 75     let chain_id = ETH_MAINNET_CHAIN_ID;
  76 
  77     // Step 3 (SignedKeyRequest metadata validation): verify the ABI-encoded metadata blob,
  78     // then compare the recovered `requestSigner` against the custody address on file for
  79     // `requestFid`. The first is pure crypto; the second is a storage lookup and lives

   77     // Step 3 (SignedKeyRequest metadata validation): verify the ABI-encoded metadata blob,
   78     // then compare the recovered `requestSigner` against the custody address on file for
   79     // `requestFid`. The first is pure crypto; the second is a storage lookup and lives
   80     // here — keeping `core::validations` free of storage deps (see NEYN-10570 note).
!  81     let verified = verify_signed_key_request_metadata(
!  82         key_add_body.metadata_type,
!  83         &key_add_body.metadata,
!  84         &key_add_body.key,
!  85         current_timestamp,
!  86         chain_id,
!  87     )?;
   88 
!  89     let request_fid_event = onchain_event_store
!  90         .get_id_register_event_by_fid(verified.request_fid, Some(txn_batch))
!  91         .map_err(|_| MessageValidationError::MissingFid)?
!  92         .ok_or(ValidationError::InvalidSignedKeyRequest)?;
!  93     let request_fid_custody = match request_fid_event.body {
!  94         Some(proto::on_chain_event::Body::IdRegisterEventBody(b)) => b,
!  95         _ => return Err(ValidationError::InvalidSignedKeyRequest.into()),
   96     };
!  97     if request_fid_custody.to.as_slice() != verified.request_signer.as_slice() {
!  98         return Err(ValidationError::SignedKeyRequestCustodyMismatch.into());
!  99     }
  100 
  101     // Step 5 (EIP-712 custody recovery for this FID's KeyAdd payload). The recovered
  102     // address must match the custody address currently on file for `fid`.
! 103     let payload = KeyAddPayload {
! 104         fid,
! 105         key: &key_add_body.key,
! 106         key_type: key_add_body.key_type,
! 107         scopes: &key_add_body.scopes,
! 108         ttl: key_add_body.ttl,
! 109         nonce: key_add_body.nonce,
! 110         deadline: key_add_body.deadline,
! 111     };
! 112     let recovered =
! 113         recover_key_add_custody_address(&payload, &key_add_body.custody_signature, chain_id)?;
! 114     let this_fid_event = onchain_event_store
! 115         .get_id_register_event_by_fid(fid, Some(txn_batch))
! 116         .map_err(|_| MessageValidationError::MissingFid)?
! 117         .ok_or(MessageValidationError::MissingFid)?;
! 118     let this_fid_custody = match this_fid_event.body {
! 119         Some(proto::on_chain_event::Body::IdRegisterEventBody(b)) => b,
! 120         _ => return Err(MessageValidationError::MissingFid),
  121     };
! 122     if this_fid_custody.to.as_slice() != recovered.as_slice() {
! 123         return Err(ValidationError::InvalidSignature.into());
! 124     }
  125 
  126     // Step 8 (conflict resolution). Gasless-first per design — writes go to the gasless
  127     // store first, so reads match the write order for any future invariant checks. Both
  128     // branches return the same typed variant; callers don't need to distinguish which

  126     // Step 8 (conflict resolution). Gasless-first per design — writes go to the gasless
  127     // store first, so reads match the write order for any future invariant checks. Both
  128     // branches return the same typed variant; callers don't need to distinguish which
  129     // index rejected them — "you can't add this key again" is the actionable signal.
! 130     if exists_gasless_key(db, txn_batch, fid, &key_add_body.key)? {
! 131         return Err(ValidationError::KeyAlreadyRegistered.into());
! 132     }
! 133     let existing_onchain = onchain_event_store
! 134         .get_active_signer(fid, key_add_body.key.clone(), Some(txn_batch))
! 135         .map_err(|_| MessageValidationError::MissingSigner)?;
! 136     if existing_onchain.is_some() {
! 137         return Err(ValidationError::KeyAlreadyRegistered.into());
! 138     }
  139 
  140     // Step 4 (nonce CAS). Stages the bump on txn_batch; rolls back with everything else
  141     // if any later step fails. The store rejects `new_nonce <= stored`, which implicitly
  142     // covers `nonce == 0` (stored defaults to 0).
! 143     check_and_set_user_nonce(db, txn_batch, fid, key_add_body.nonce)?;
  144 
  145     // State writes. Record embeds the full message (source of truth) plus the cached
  146     // request_fid. last_used_at is initialized to the message's own timestamp so the
  147     // sliding-TTL window begins at creation.
! 148     let record = GaslessKeyRecord {
! 149         message: Some(msg.clone()),
! 150         request_fid: verified.request_fid,
! 151     };
! 152     put_gasless_key_record(db, txn_batch, fid, &key_add_body.key, &record)?;
! 153     init_last_used_at(
! 154         db,
! 155         txn_batch,
! 156         fid,
! 157         &key_add_body.key,
! 158         message_data.timestamp,
! 159     )?;
  160 
! 161     Ok(HubEvent::new_event(
! 162         HubEventType::MergeMessage,
! 163         hub_event::Body::MergeMessageBody(proto::MergeMessageBody {
! 164             message: Some(msg.clone()),
! 165             deleted_messages: vec![],
! 166         }),
! 167     ))
! 168 }
  169 
  170 /// Orchestrates the KEY_REMOVE flow for NEYN-10574: deactivates a gasless key under one of two
  171 /// authorization modes, then clears the sibling `last_used_at` entry.
  172 ///

  199 /// across both modes for client libraries, at no cost here.
  200 ///
  201 /// On-chain signers are intentionally out of scope — they are removed by on-chain
  202 /// `SIGNER_EVENT_TYPE_REMOVE` events via the Key Registry contract, not by this path.
! 203 pub fn merge_key_remove(
! 204     db: &Arc<RocksDB>,
! 205     onchain_event_store: &OnchainEventStore,
! 206     msg: &proto::Message,
! 207     txn_batch: &mut RocksDbTransactionBatch,
! 208 ) -> Result<HubEvent, MessageValidationError> {
! 209     let message_data = msg
! 210         .data
! 211         .as_ref()
! 212         .ok_or(MessageValidationError::NoMessageData)?;
! 213     let fid = message_data.fid;
! 214     let key_remove_body = match &message_data.body {
! 215         Some(Body::KeyRemoveBody(body)) => body,
  216         _ => {
! 217             return Err(MessageValidationError::InvalidMessageType(
! 218                 message_data.r#type,
! 219             ))
  220         }
  221     };
! 222     let chain_id = ETH_MAINNET_CHAIN_ID;
  223 
  224     // Active-key lookup. Does triple duty: existence check, request_fid source for
  225     // self-revocation, and prior-message source for the emitted event's deleted_messages.
! 226     let record = get_gasless_key_record(db, txn_batch, fid, &key_remove_body.key)?
! 227         .ok_or(ValidationError::KeyNotRegistered)?;
  228 
  229     // `?` here is belt-and-suspenders: `validate_user_message` already ran
  230     // `validate_key_remove_body`, which rejects unknown discriminants via the same TryFrom. If
  231     // that upstream contract is ever bypassed (e.g. a future internal injection path), this

  230     // `validate_key_remove_body`, which rejects unknown discriminants via the same TryFrom. If
  231     // that upstream contract is ever bypassed (e.g. a future internal injection path), this
  232     // re-check keeps the function safe in isolation. Exhaustive match below means adding a
  233     // future variant forces us to update this site — the constant-based match couldn't.
! 234     let sig_type = KeyRemoveSignatureType::try_from(key_remove_body.signature_type)?;
! 235     match sig_type {
  236         KeyRemoveSignatureType::Custody => {
! 237             let payload = KeyRemovePayload {
! 238                 fid,
! 239                 key: &key_remove_body.key,
! 240                 nonce: key_remove_body.nonce,
! 241                 deadline: key_remove_body.deadline,
! 242             };
! 243             let recovered =
! 244                 recover_key_remove_custody_address(&payload, &key_remove_body.signature, chain_id)?;
! 245             let this_fid_event = onchain_event_store
! 246                 .get_id_register_event_by_fid(fid, Some(txn_batch))
! 247                 .map_err(|_| MessageValidationError::MissingFid)?
! 248                 .ok_or(MessageValidationError::MissingFid)?;
! 249             let this_fid_custody = match this_fid_event.body {
! 250                 Some(proto::on_chain_event::Body::IdRegisterEventBody(b)) => b,
! 251                 _ => return Err(MessageValidationError::MissingFid),
  252             };
! 253             if this_fid_custody.to.as_slice() != recovered.as_slice() {
! 254                 return Err(ValidationError::InvalidSignature.into());
! 255             }
! 256             check_and_set_user_nonce(db, txn_batch, fid, key_remove_body.nonce)?;
  257         }
  258         KeyRemoveSignatureType::SelfRevoke => {
  259             // Self-revocation. The envelope signer is the key being revoked, and the envelope
  260             // Ed25519 signature was already verified upstream. Asserting signer == body.key

  259             // Self-revocation. The envelope signer is the key being revoked, and the envelope
  260             // Ed25519 signature was already verified upstream. Asserting signer == body.key
  261             // is all that's left — any other signer means this message is not authorized by
  262             // the key's holder.
! 263             if msg.signer.as_slice() != key_remove_body.key.as_slice() {
! 264                 return Err(ValidationError::InvalidSignature.into());
! 265             }
! 266             check_and_set_app_nonce(db, txn_batch, record.request_fid, key_remove_body.nonce)?;
  267         }
  268     }
  269 
  270     // State writes. Order: record first (the authoritative existence signal), then

  269 
  270     // State writes. Order: record first (the authoritative existence signal), then
  271     // last_used_at (cleanup of the sibling store). Both are per-(fid, key) so no cross-key
  272     // interleaving concerns.
! 273     delete_gasless_key_record(db, txn_batch, fid, &key_remove_body.key)?;
! 274     delete_last_used_at(db, txn_batch, fid, &key_remove_body.key)?;
  275 
! 276     Ok(HubEvent::new_event(
! 277         HubEventType::MergeMessage,
! 278         hub_event::Body::MergeMessageBody(proto::MergeMessageBody {
! 279             message: Some(msg.clone()),
! 280             deleted_messages: record.message.into_iter().collect(),
! 281         }),
! 282     ))
! 283 }

src/storage/store/block_engine.rs

  282                 .onchain_event_store
  283                 .get_active_signer(message_data.fid, message.signer.clone(), Some(txn_batch))
  284                 .map_err(|_| MessageValidationError::MissingSigner)?
  285                 .ok_or(MessageValidationError::MissingSigner)?;
! 286         }
  287 
  288         match message_data
  289             .body
  290             .as_ref()

  319             // engine level: static body validation (key length, scopes, ttl bound, etc.) ran
  320             // upstream in `validate_message`, and state-dependent checks (nonce CAS, custody
  321             // recovery, conflict resolution) live in the merge helpers themselves.
  322             crate::proto::message_data::Body::KeyAddBody(_)
! 323             | crate::proto::message_data::Body::KeyRemoveBody(_) => {}
  324             _ => return Err(MessageValidationError::InvalidMessageType),
  325         }
  326 
  327         Ok(())

  337                 &self.stores.storage_lend_store,
  338                 message,
  339                 txn_batch,
  340             )?),
! 341             MessageType::KeyAdd => Ok(vec![crate::storage::store::account::merge_key_add(
! 342                 &self.stores.db,
! 343                 &self.stores.onchain_event_store,
! 344                 message,
! 345                 txn_batch,
! 346             )?]),
! 347             MessageType::KeyRemove => Ok(vec![crate::storage::store::account::merge_key_remove(
! 348                 &self.stores.db,
! 349                 &self.stores.onchain_event_store,
! 350                 message,
! 351                 txn_batch,
! 352             )?]),
  353             _ => return Err(MessageValidationError::InvalidMessageType),
  354         }
  355     }

  444                     MessageType::KeyAdd | MessageType::KeyRemove => {
  445                         // No storage-slot accounting needed — gasless keys don't consume
  446                         // storage units. Emitted MergeMessageBody propagates to shards via
  447                         // BlockEvent so their local DBs can replay the same merge.
! 448                         if let Ok(events) = self.merge_message(message, txn_batch) {
! 449                             hub_events.extend(events);
! 450                         }
  451                     }
  452                     _ => {}
  453                 },
  454                 Err(err) => {

  1080             MessageValidationError::HubError(h) => {
  1081                 assert_eq!(h.code, source.code);
  1082                 assert_eq!(h.message, source.message);
  1083             }
! 1084             other => panic!("expected HubError, got {other:?}"),
  1085         }
  1086     }
  1087 
  1088     #[test]

  1095         match out {
  1096             MessageValidationError::HubError(h) => {
  1097                 assert!(
  1098                     h.message.contains("fname"),
! 1099                     "expected source message to carry 'fname', got: {}",
  1100                     h.message
  1101                 );
  1102             }
! 1103             other => panic!("expected HubError fallthrough, got {other:?}"),
  1104         }
  1105     }
  1106 }

src/storage/store/engine.rs

  1223                 let store = &self.stores.storage_lend_store;
  1224                 StorageLendStore::merge(store, msg, txn_batch)
  1225                     .map_err(|e| MessageValidationError::StoreError(e))?
  1226             }
! 1227             MessageType::KeyAdd => vec![crate::storage::store::account::merge_key_add(
! 1228                 &self.db,
! 1229                 &self.stores.onchain_event_store,
! 1230                 msg,
! 1231                 txn_batch,
! 1232             )?],
! 1233             MessageType::KeyRemove => vec![crate::storage::store::account::merge_key_remove(
! 1234                 &self.db,
! 1235                 &self.stores.onchain_event_store,
! 1236                 msg,
! 1237                 txn_batch,
! 1238             )?],
  1239             unhandled_type => {
  1240                 return Err(MessageValidationError::InvalidMessageType(
  1241                     unhandled_type as i32,
  1242                 ));

@topocount topocount enabled auto-merge (squash) April 24, 2026 12:24
@topocount topocount merged commit dfabd64 into main Apr 24, 2026
10 checks passed
@topocount topocount deleted the kjs/neyn-10580-shard-0-routing-for-key-messages branch April 24, 2026 12:30
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