Cosmos-sdk: Signatures converted from LegacyAmino to SignaturesV2 fail on sequence number other than 0

[zmanian]

https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/types/stdtx.go#L381-L393

This converts legacy amino signatures to the new SignaturesV2. This always sets SignaturesV2 Sequence to 0.

Here we check if the account sequence matches matches the onchain sequence number.

https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/ante/sigverify.go#L210-L215

This means signatures from that are converted from LegacyAmino will always fail in the ante handler except for the 1st tx from an account.

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[zmanian]

on possible suggestion is that we skip sig.Sequence != acc.GetSequence() check if sig.Sequence == 0 && acc.GetSequence() != 0

[amaurymartiny]

From Zaki:

Okay so this fix didn't actually work.

What happened is that CopyTx extracts a V2 Signature with SkipSequenceCheck on it but then immediately puts in to SignerInfos that doesn't contain SkipSequenceCheck field.

When the anteHandler calls GetSignatureV2(), it gets back a Signature with SkipSequenceCheck as false.
https://github.com/cosmos/cosmos-sdk/blob/master/client/tx/legacy.go#L39-L60

[alexanderbez]

So do we need to instead add SkipSequenceCheck to SignerInfos?

[robert-zaremba]

I will try to look at it. @zmanian , @amaurymartiny - do we have some test cases for this? How can I replicate? Or I just need to dive in the code and guess what's wrong?

[aaronc]

I just discussed this with @amaurymartiny.

I don't think we actually need SkipSequenceCheck at all. We just need to skip the sequence check if SIGN_MODE_LEGACY_AMINO_JSON is used. For multisigs, this would mean that we skip the check only if all signers use legacy amino JSON.