-
Notifications
You must be signed in to change notification settings - Fork 218
Add multi-sig example binaries #144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@lmlmt note that multi-sig conversion isn’t implemented in the Rust SDK yet; the examples in this PR are scaffolding to show the shape/params. Once this PR is merged, I can look into adding the actual conversion support (or exposing a minimal API surface) and follow up with a dedicated PR |
@nicolad prefer having the multi-sig functionality implemented as well, for completeness |
thank you @lmlmt |
- convert_to_multi_sig_user: Convert user to multi-sig with authorized users and threshold - multi_sig_order: Execute multi-sig orders with JSON and typed actions - multi_sig_register_token: Multi-sig token registration with spot deploy - multi_sig_usd_send: Multi-sig USD transfers with signature chain parameters Examples show structure for future implementation when multi-sig functionality is added to Rust SDK
…unt conversion, address management, and transaction operations for orders, USDC transfers, and spot transfers including comprehensive tests and examples.
luca992
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need multi-sign support so I was going over this pr, but I think the final payload is incorrect.
| async fn post_multi_sig( | ||
| &self, | ||
| action: serde_json::Value, | ||
| signatures: Vec<Signature>, | ||
| nonce: u64, | ||
| ) -> Result<ExchangeResponseStatus> { | ||
| let exchange_payload = MultiSigExchangePayload { | ||
| action, | ||
| signatures, | ||
| nonce, | ||
| vault_address: self.vault_address, | ||
| }; | ||
| let res = serde_json::to_string(&exchange_payload) | ||
| .map_err(|e| Error::JsonParse(e.to_string()))?; | ||
| debug!("Sending multi-sig request {res:?}"); | ||
|
|
||
| let output = &self | ||
| .http_client | ||
| .post("/exchange", res) | ||
| .await | ||
| .map_err(|e| Error::JsonParse(e.to_string()))?; | ||
| debug!("Response: {output}"); | ||
| serde_json::from_str(output).map_err(|e| Error::JsonParse(e.to_string())) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you are creating this correctly @nicolad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, appreciate the reference, will adjust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please have another look, adjusted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolad I'll actually try it, but looks like it will work if you have access to all the signers which is fine in my case. But this isn't something you can assume in most multisig cases. I think you'd want the function to take Signatures and not PrivateKeySigners? Also might need a way to export a signature. I'd think the usual flow is you collect signatures from the other wallets and when you have enough you as the submitter add your last signature and attach all the other signatures you collected.
This is what the python sdk above appears to do.
pub async fn multi_sig_usdc_transfer(
&self,
multi_sig_user: Address,
amount: &str,
destination: &str,
wallets: &[PrivateKeySigner],
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thank you.
src/exchange/actions.rs
Outdated
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub payload_multi_sig_user: Option<String>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub outer_signer: Option<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These shouldn't be here. The hyperliquid api returns a deserializaiton an error if you include these
I fixed it in nicolad#1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work @luca992, merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now hope @lmlmt will get a chance to 👀 over this PR
remove SendAsset payload_multi_sig_user & outer_signer
This reverts commit 5d81c38.
This causes issues especially with message packing multi-sig transaction
when for example a serde Value with a number will look like this when packed:
`0x83b07369676e6174757265436861696e4964a3307831aa7369676e6174757265739183a172d942307864623433373135643037383931613063363361383937376139393534646332346664326663363063373436643734353566363133326236396163396665393734a173d942307832386236323363376364643563383230643461663832326634323336623361663361613063373938653736396235353031613331323162343634623761353835a17681bc2473657264655f6a736f6e3a3a707269766174653a3a4e756d626572a23238a77061796c6f616483ac6d756c746953696755736572d92a307832623339633062636664343763356132376636333766383832303661666239356637616661323838ab6f757465725369676e6572d92a307862373131613133636261663734316138383263373230623433363861346566323661346631613632a6616374696f6e8aa474797065a973656e644173736574b07369676e6174757265436861696e4964a730783636656565b068797065726c6971756964436861696ea7546573746e6574ab64657374696e6174696f6ed92a307836643938383538373139363035643332366664633136343839363737333432343462323762386133a9736f75726365446578a0ae64657374696e6174696f6e446578a0a5746f6b656ea455534443a6616d6f756e74a4302e3031ae66726f6d5375624163636f756e74a0a56e6f6e636581bc2473657264655f6a736f6e3a3a707269766174653a3a4e756d626572ad31373632383338313437303030`
```
{
"signatureChainId": "0x1",
"signatures": [
{
"r": "0xdb43715d07891a0c63a8977a9954dc24fd2fc60c746d7455f6132b69ac9fe974",
"s": "0x28b623c7cdd5c820d4af822f4236b3af3aa0c798e769b5501a3121b464b7a585",
"v": {
"$serde_json::private::Number": "28"
}
}
],
"payload": {
"multiSigUser": "0x2b39c0bcfd47c5a27f637f88206afb95f7afa288",
"outerSigner": "0xb711a13cbaf741a882c720b4368a4ef26a4f1a62",
"action": {
"type": "sendAsset",
"signatureChainId": "0x66eee",
"hyperliquidChain": "Testnet",
"destination": "0x6d98858719605d326fdc1648967734244b27b8a3",
"sourceDex": "",
"destinationDex": "",
"token": "USDC",
"amount": "0.01",
"fromSubAccount": "",
"nonce": {
"$serde_json::private::Number": "1762838147000"
}
}
}
}
```
This reverts commit b79d86a.
This reverts commit c660e12.
This reverts commit 700679d.
This reverts commit 5ef467f.
luca992
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side note the multisig apis you made are using SendAsset not UsdSend... it works (You can use either to send usd) but is not really named correctly.
|
|
||
| let mut bytes = rmp_serde::to_vec_named(&action_without_type) | ||
| .map_err(|e| Error::RmpParse(e.to_string()))?; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it working, but had to make a lot of fixes.
Also had to add back payload_multi_sig_user & outer_signer but with serialization disabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @luca992, isn't this the beauty of OS contribution? Thx.
Multi sig fixes
Related to: #143