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

Implement correctly cosmos account sequence #1334

Closed
NicolasMahe opened this issue Sep 12, 2019 · 7 comments · Fixed by #1462
Closed

Implement correctly cosmos account sequence #1334

NicolasMahe opened this issue Sep 12, 2019 · 7 comments · Fixed by #1462
Assignees
Labels
bug Something isn't working

Comments

@NicolasMahe
Copy link
Member

Conversation started on #1267 (comment).

The account sequence is currently hardcoded and only work when transactions have different content.
But if 2 transactions have the same content, then they generate the same hash and thus the second is rejected.

The sequence should be always be different.

  • Must the sequence number be incremented?
  • Maybe use the last block height at a starting point?
@krhubert
Copy link
Contributor

@antho1404
Copy link
Member

We should use the GetSequence from the AccountKeeper https://github.com/cosmos/cosmos-sdk/blob/fb3e0af0b6320838c1da8531deba6c6a542c376a/x/auth/keeper/keeper.go#L61

@NicolasMahe
Copy link
Member Author

I think the Account SDK should return data from the Account Keeper. It looks like the AccountKeeper has all the data needed.
If we do this, then the functions NewTxBuilder and client.BuildAndBroadcastMsg could also accept the AccountKeeper to directly get the sequence, and account number ;)

@krhubert
Copy link
Contributor

krhubert commented Nov 5, 2019

Will use GetSequence and GetAccountNumber then.

@NicolasMahe
Copy link
Member Author

NicolasMahe commented Nov 5, 2019

The SetSequence is already managed by cosmos when applying a transaction:

https://github.com/cosmos/cosmos-sdk/blob/02c6c9fafd58da88550ab4d7d494724a477c8a68/x/auth/ante/sigverify.go#L252

@antho1404
Copy link
Member

With that, we should be able to simply use GetSequence all the time and not have to manage the sequence at all.

@krhubert
Copy link
Contributor

krhubert commented Nov 5, 2019

This is not that simple. We need to have cosmos context to get the account.

I've used auth.NewAccountRetriver to get account number and sequence via rest api.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants