Skip to content
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

Better error for failed signature verification #3655

Closed
jackzampolin opened this issue Feb 14, 2019 · 15 comments · Fixed by #4395
Closed

Better error for failed signature verification #3655

jackzampolin opened this issue Feb 14, 2019 · 15 comments · Fixed by #4395

Comments

@jackzampolin
Copy link
Member

This error can normally arises from an incorrect sequence number (trying to send another transaction too quickly) or chain_id

@faboweb
Copy link
Contributor

faboweb commented May 21, 2019

Push

@alexanderbez
Copy link
Contributor

What do we want here? Just to update the error message?

How about: signature verification failed; verify correct account sequence and chain-id.

@faboweb
Copy link
Contributor

faboweb commented May 21, 2019

It would be cool if it would say if this is an error based on the wrong sequence or the wrong chain-id

@alexanderbez
Copy link
Contributor

@faboweb this is not possible afaik. We construct bytes over which we expect the signer to sign and then call pubKey.VerifyBytes(signBytes, sig.Signature). We have no idea what the user/client provided.

@faboweb
Copy link
Contributor

faboweb commented May 21, 2019

But you receive the whole message which holds not only the signature but the chain-id and the sequence number. Can't those be evaluated beforehand?

@alexanderbez
Copy link
Contributor

alexanderbez commented May 21, 2019

But you receive the whole message which holds not only the signature but the chain-id and the sequence number. Can't those be evaluated beforehand?

Transactions do not contain the chain-id or anything related to the account. This was removed some time ago. It's not going to be added back as it's redundant and would bloat state too much.

@faboweb
Copy link
Contributor

faboweb commented May 22, 2019

Ahh you're right, I still had the old format in my memory.
Then signature verification failed; verify correct account sequence and chain-id sounds good.

Who is evaluating the tx? The full node which is on a certain chain-id? So if I know the full nodes chain-id I could probably exclude the chain-id as a source of error.

Spinning this further: How about sending the chain-id and sequence together with the tx (even though they are not included in the tx that ends up in the state) in the broadcast request. The full node could then evaluate these optional arguments and return a proper error.

@alexanderbez
Copy link
Contributor

Who is evaluating the tx? The full node which is on a certain chain-id? So if I know the full nodes chain-id I could probably exclude the chain-id as a source of error.

You'd be surprised haha. But with the advent of more mature wallets, I agree this should be less of a common problem.

Spinning this further: How about sending the chain-id and sequence together with the tx (even though they are not included in the tx that ends up in the state) in the broadcast request. The full node could then evaluate these optional arguments and return a proper error.

Hmmm, I suppose this would be nice, but I don't know of anyway of doing this because the ante-handler receives a StdTx. There is no notion of optional arguments per say...at least that are not persisted.

The best I think we can do here is improve the error message.

@faboweb
Copy link
Contributor

faboweb commented May 22, 2019

I thought about a check in the handler of the broadcast endpoint. Would that be a possibility?

@valuead
Copy link

valuead commented Jul 24, 2019

image
still persist
Run 2 commands one after another in bash file script and this is what you get. Will try to include wait for a sec. ))

@alexanderbez
Copy link
Contributor

Yes, indeed @valuead. What are you stating?

@valuead
Copy link

valuead commented Jul 24, 2019

By adding 10 seconds between commands in my bash script error was gone or was occurring rarely so next command was bonding my tokens. (C) Maestro

@alexanderbez
Copy link
Contributor

Yes, that is due to the fact of the CheckTx state not having the correct sequence. By waiting for the next block, you'll have the correct value.

@yzhanginwa
Copy link

Yes, that is due to the fact of the CheckTx state not having the correct sequence. By waiting for the next block, you'll have the correct value.

So each account/address can send 1 transaction the most in a block, or the 2nd transaction will get a duplicate sequence number. Is that what you mean?

@alexanderbez
Copy link
Contributor

You should send 1 tx with multiple messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants