-
Notifications
You must be signed in to change notification settings - Fork 123
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
Add alternate type for proto::cosmos::tx::v1beta1::TxRaw that is compatible with ledger. #309
Comments
Perhaps both body and auth_info will need different implementations as well. |
To my knowledge the only format the Ledger supports is Amino JSON, which predates and is orthogonal to Protobufs. There's no support for Amino in this crate. If there's enough interest we could support Amino JSON, but that's a question for #309 |
FWIW, I wrote an Amino library a couple years ago, though it was mainly aimed at the binary format: https://github.com/iqlusioninc/crates/tree/main/stdtx It does have some JSON support, although at this point I couldn't tell you what state it's in. |
So the problem I'm having in particular is how to package up the TxRaw after signing. I can properly get the json payload to the ledger and sign it, but I don't know how I can then take the signature that's returned, and bundle it into TxRaw. There's very little documentation around for me to go off of so I'm wandering in the dark a bit haha. |
That library could be helpful, but Everything I've written so far is dependent on cosmrs, and I'd like to avoid branching out if possible. Do you not think it would be possible to accomplish this with cosmrs? |
You need to use the I believe the transaction will need to be encoded in the Amino binary wire format (even though the signature is over the JSON). The easiest way to confirm any of this would be to find a working transaction and use |
That's very helpful thank you. I'll look into that right now!
Could you elaborate on that idea a little more? |
Find the Protobuf binary serialization of any working transaction signed by a Ledger and parse that. We can add it to the test vectors if you'd like. I can look for one if you want. |
Sure if you'd like to help! That would be great. |
I switched to using the legacy amino sign mode but my transactions are still failing the signature verification step. |
I believe that's because they need to be serialized using the Amino binary encoding. Are you doing that? |
Steps:
{
"account_number": {number},
"chain_id": {string},
"fee": {
"amount": [{"amount": {number}, "denom": {string}}, ...],
"gas": {number}
},
"memo": {string},
"msgs": [{arbitrary}],
"sequence": {number}
} the data is returned and converted to a signature via Signature::from_der(data.as_slice()) Then I build the TxRaw let anys = msgs.iter().map(|msg| msg.to_any()).collect::<Result<Vec<_>, _>>().unwrap();
let tx_body = tx::Body::new(anys, memo.clone(), timeout_height);
let signer_info = SignerInfo {
public_key: Some(public_key.into()),
mode_info: ModeInfo::Single(Single { mode: SignMode::LegacyAminoJson }),
sequence: account.sequence,
};
let ledger_auth_info = AuthInfo { signer_infos: vec![signer_info], fee: fee.clone() };
let raw = cosmos_sdk_proto::cosmos::tx::v1beta1::TxRaw {
body_bytes: tx_body.into_bytes().unwrap(),
auth_info_bytes: ledger_auth_info.into_bytes().unwrap(),
signatures: vec![signed.to_vec()],
} |
at what point in the process are you suggesting it be serialized using the Amino binary encoding? let tx_commit_response = tx_raw.broadcast_commit(client).await?; to broadcast |
FWIW here's
I don't know it's actually doing that. The transaction is hopefully still Protobuf even though it's using Amino JSON for signing. I'm just guessing here. This is why we need an example transaction which uses |
Not exactly sure the easiest way to get something like that. I'm so close to making this work this is literally the last step I'm stuck on ahah. |
Once you get the signature how do you then broadcast with that library? |
You won't be able to with It will need to use https://docs.rs/cosmrs/latest/cosmrs/tx/struct.Raw.html#method.broadcast_commit, but you'll have to actually solve this issue before that will work |
sounds good to me. Well thanks for your help on this issue! Really appreciate it. If you come up with any other ideas for how to get this to work let me know. This is the repository I'm currently working on for reference. |
Following up here! Did some digging and found that tmkms uses your stdtx package to solve exactly this problem. Examples can be found in tx_signer.rs. I'll be using that as an example, and hopefully I can get this all sorted. |
@tony-iqlusion you mentioned above that it might be possible to get amino support within cosmrs and I think that's exactly what my use case requires! I'll try and build around that for now, but as a long term goal that would be fantastic. |
I could be a little off base here as my understanding of this is pretty shallow at best. Ledgers expect a specific version of "SignDoc" that must have all json objects sorted by key. you can see the spec here. It seems to me that that would requires a different form for Raw, as the current implementation separates "auth_info" and "body". When these are passed to the ledger, they must first be sorted. The sorted version is signed, and then transforming back into Raw causes a jumble, and invalidates the signature. Is my understanding of this problem correct?
The text was updated successfully, but these errors were encountered: