fix: set signatureChainId in recover_user_from_user_signed_action - #312
Conversation
sign_user_signed_action sets both signatureChainId and hyperliquidChain on the action, but recover_user_from_user_signed_action only set hyperliquidChain. Since user_signed_payload reads action["signatureChainId"], recovering the signer from a freshly reconstructed action raised KeyError: 'signatureChainId'. Set signatureChainId symmetrically with sign_user_signed_action, and add a sign -> recover round-trip regression test (the function had no coverage).
koriyoshi2041
left a comment
There was a problem hiding this comment.
Verified at a08ae6e761. A freshly reconstructed USD-send action fails with KeyError: 'signatureChainId' on master, while this restores the same EIP-712 domain input used by sign_user_signed_action; mainnet and testnet round trips recover the signer correctly. The full local suite passes (37/37) on Python 3.11, and git diff --check is clean.
The hardcoded value deliberately matches current master. If #267 later makes it configurable, both sign and recover paths will need to move together.
|
Thanks for the thorough verification — reproducing the KeyError on master and running the round trips on both chains is more than most PRs get. Agreed on the #267 coupling: the hardcoded value deliberately mirrors current master, and if it becomes configurable the sign and recover paths have to move together. Happy to update this PR if that lands first. |
Summary
recover_user_from_user_signed_actionis the inverse ofsign_user_signed_action, but the two are asymmetric:sign_user_signed_actionsets bothsignatureChainIdandhyperliquidChainon the action before building the EIP-712 payload.recover_user_from_user_signed_actionsets onlyhyperliquidChain.Because
user_signed_payloadreadsaction["signatureChainId"], recovering the signer from a freshly reconstructed action — the normal case for a verifier that never calledsign_user_signed_actionon that dict — raisesKeyError: 'signatureChainId'.Reproduction
Fix
Set
signatureChainIdinrecover_user_from_user_signed_actionsymmetrically withsign_user_signed_action. Added a sign → recover round-trip regression test (mainnet + testnet) that asserts the recovered address equals the signer — the function previously had no test coverage, which is why this slipped through.pytest tests/signing_test.pypasses (14 tests).Note on #267
This is orthogonal to #267 (which makes
signatureChainIdconfigurable insign_user_signed_action). The fix here matches currentmaster, wheresign_user_signed_actionhardcodes0x66eee. If #267 lands, the recover side should stay in sync — ideally both sign and recover would share a single helper for the default so they can't drift apart again.