Skip to content

Conversation

@Kailai-Wang
Copy link
Collaborator

@Kailai-Wang Kailai-Wang commented Nov 4, 2025

Context

This is a big refactoring to the current response and error handling implementation:

  • remove PumpxRpcError completely and use DetailedError instead as the only wrapper
  • greatly reduce the error codes, see error_code.rs
  • use jsonrpsee raw error_code and types when possible
  • write many helpers to reduce verbosity/duplicate and enhance readability, see detailed_error.rs, RpcResultExt, RpcOptionExt, extract_omni_account, parse_as, parse_rpc_params ... etc

As result, more than 2000 LoC can be removed:

with 2,155 additions and 4,490 deletions.

I think some places could be further improved, e.g. verify_auth, utils/auth.rs.

@vercel
Copy link

vercel bot commented Nov 4, 2025

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

Project Deployment Preview Comments Updated (UTC)
heima Ready Ready Preview Comment Nov 5, 2025 9:58am
heima-aa-demo-app Ready Ready Preview Comment Nov 5, 2025 9:58am

@claude
Copy link

claude bot commented Nov 4, 2025

Claude encountered an error —— View job


PR Review: Simplify RPC Response and Error Handling

I'm conducting a comprehensive review of this major refactoring. Here's my progress:

Review Tasks

  • Analyze the new error handling architecture (DetailedError vs PumpxRpcError)
  • Review error code simplification changes
  • Examine new utility modules (auth.rs, validation.rs, types.rs)
  • Check method implementations for proper pattern usage
  • Assess security implications of the changes
  • Evaluate performance considerations
  • Check for potential bugs or regressions
  • Provide specific feedback with line references

Initial Findings

Critical Issue Found:

  • Bug in error_code.rs:17 - WILDMETA_SERVICE_ERROR_CODE has positive value 32165 instead of negative. Should be -32165 to follow JSON-RPC specification.

Continuing analysis...

Copy link
Member

@kziemianek kziemianek left a comment

Choose a reason for hiding this comment

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

I left small comments. This is a good code improvement.

"Authentication verification failed"
).with_suggestion("Please check your authentication credentials"))
})?;
debug!("Received omni_exportWallet, params: {:?}", params);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe middleware could be used for that

@Kailai-Wang Kailai-Wang merged commit 6b79190 into dev Nov 5, 2025
19 checks passed
@Kailai-Wang Kailai-Wang deleted the simplify-rpc-response branch November 5, 2025 11:09
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