Skip to content

Conversation

@Kailai-Wang
Copy link
Collaborator

@Kailai-Wang Kailai-Wang commented Oct 22, 2025

Context

Fixes P-1717

This PR tries to remove teh NativeTaskWrapper (and thus native-task-handler) completely by inlining the handling directly in rpc method handler.

With help-check of claude It tries to keep the error handling backwards compatible/same so we don't return e.g. different error code, but much of the code is not used so I'm not 100% sure if it covers all cases strictly, as I indeed find some unused error codes in error_code.rs

Some more clean-ups:

  • the client_id in middleware auth is actually not used, so I unwrap the struct User to use the underlying omni_account directly
  • omni_submitNativeTask is removed, together with task primitives definitions
  • ethereum-intent-executor and solana-intent-executor are removed from start_server and RpcContext

@linear
Copy link

linear bot commented Oct 22, 2025

@vercel
Copy link

vercel bot commented Oct 22, 2025

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

Project Deployment Preview Comments Updated (UTC)
heima-aa-demo-app Ready Ready Preview Comment Oct 22, 2025 10:44pm

@Kailai-Wang Kailai-Wang requested a review from a team October 22, 2025 09:02
@claude
Copy link

claude bot commented Oct 22, 2025

Claude encountered an error —— View job


PR Review: Refactor to remove native task wrapper

Reviewing this large refactoring PR that removes the NativeTaskWrapper layer. Here's my analysis plan:

Todo List

  • Read repository guidelines (CLAUDE.md) - Not found
  • Analyze deleted files and understand removed functionality
  • Review key changed files for implementation quality
  • Check error handling backwards compatibility
  • Review security implications of auth changes
  • Assess performance considerations
  • Check for potential bugs and edge cases
  • Verify proper cleanup of dependencies
  • Provide comprehensive feedback and recommendations

Initial Analysis

This PR makes significant changes by removing the entire native-task-handler crate (3611 lines deleted) and inlining its functionality directly into RPC method handlers. Key observations:

  1. Removed Components:

    • native-task-handler crate completely deleted
    • submit_native_task.rs method removed
    • Intent executors removed from server startup
    • Task wrapper abstraction eliminated
  2. Simplifications:

    • Auth middleware simplified to use omni_account directly instead of User struct
    • Direct handler invocation instead of task wrapper pattern
    • Consolidated error handling into RPC methods

Copy link
Contributor

@silva-fj silva-fj left a comment

Choose a reason for hiding this comment

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

LGTM

@Kailai-Wang Kailai-Wang enabled auto-merge (squash) October 22, 2025 22:42
@Kailai-Wang Kailai-Wang merged commit 72cb44f into dev Oct 22, 2025
19 checks passed
@Kailai-Wang Kailai-Wang deleted the refactor-to-remove-native-task-wrapper branch October 22, 2025 23:47
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.

3 participants