Use a single cosmos codec #1480
Conversation
antho1404
changed the title
Just wonder if two clients ask for the account sequence number at the same time, they might get the same seq number. The sequence number is only increased in one place (https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/ante/sigverify.go#L252) - on sig validation. So my question is are you sure the two clients get new sequence when they retrive account at the same time. |
sdk/ownership/codec.go
Outdated
| // ModuleCdc is the codec for the module | ||
| var ModuleCdc = codec.New() | ||
|
|
||
| func init() { |
There was a problem hiding this comment.
Can we replace all init function with explicit call?
There was a problem hiding this comment.
That's following the guidelines of cosmos. Also, we need to make sure that the codec is initialized even if the backend is not yet created. We could have some sync of the messages before we initialize the backend.
We could find a way to remove the init and have a singleton pattern but that will be complex for no real advantage.
There was a problem hiding this comment.
That's following the guidelines of cosmos
Yes, i think cosmos is not the right place to get best pracitices.
Also, we need to make sure that the codec is initialized even if the backend
That's easy, we can call InitCodes inside main function.
Liek for me init should be avoided at all cost, because it changes the workflow of execution. There are only few places where it should be used (like goto, or brake/continue outer loops)
There was a problem hiding this comment.
Yes, i think cosmos is not the right place to get best pracitices
Maybe not but protobuf or gogoproto might be ;)
Anyway we could have a function that the is called on every package and make sure that this is called at the beginning of main but at the end that's similar to init.
antho1404
force-pushed
the
fix/codec-ante-handler
branch
from
da7dfec to
58cf150
Compare
antho1404
force-pushed
the
fix/codec-ante-handler
branch
2 times, most recently
from
a88566d to
05279de
Compare
antho1404
changed the title
antho1404
changed the title
| // RegisterConcrete https://godoc.org/github.com/tendermint/go-amino#Codec.RegisterConcrete | ||
| func RegisterConcrete(o interface{}, name string, copts *amino.ConcreteOptions) { | ||
| Codec.RegisterConcrete(o, name, copts) | ||
| } | ||
|
|
||
| // RegisterInterface https://godoc.org/github.com/tendermint/go-amino#Codec.RegisterInterface | ||
| func RegisterInterface(ptr interface{}, iopts *amino.InterfaceOptions) { | ||
| Codec.RegisterInterface(ptr, iopts) | ||
| } | ||
|
|
||
| // MustMarshalJSON https://godoc.org/github.com/tendermint/go-amino#Codec.MustMarshalJSON | ||
| func MustMarshalJSON(o interface{}) []byte { | ||
| return Codec.MustMarshalJSON(o) | ||
| } | ||
|
|
||
| // MarshalJSON https://godoc.org/github.com/tendermint/go-amino#Codec.MarshalJSON | ||
| func MarshalJSON(o interface{}) ([]byte, error) { | ||
| return Codec.MarshalJSON(o) | ||
| } | ||
|
|
||
| // UnmarshalJSON https://godoc.org/github.com/tendermint/go-amino#Codec.UnmarshalJSON | ||
| func UnmarshalJSON(bz []byte, ptr interface{}) error { | ||
| return Codec.UnmarshalJSON(bz, ptr) | ||
| } | ||
|
|
||
| // UnmarshalBinaryBare https://godoc.org/github.com/tendermint/go-amino#Codec.UnmarshalBinaryBare | ||
| func UnmarshalBinaryBare(bz []byte, ptr interface{}) error { | ||
| return Codec.UnmarshalBinaryBare(bz, ptr) | ||
| } | ||
|
|
||
| // MarshalBinaryBare https://godoc.org/github.com/tendermint/go-amino#Codec.MarshalBinaryBare | ||
| func MarshalBinaryBare(o interface{}) ([]byte, error) { | ||
| return Codec.MarshalBinaryBare(o) | ||
| } |
There was a problem hiding this comment.
Do we need those functions? What are the purpose of those?
Why caller just can't use
codec.Codec.XXXThere was a problem hiding this comment.
What would be good to see for example is shortcut for
cosmostypes.MustSortJSON(codec.MustMarshalJSON(msg))
But it's an option
There was a problem hiding this comment.
we can use codec.Codec.XXX I just find it more convenient to have the functions directly like an alternative codec (that in fact uses another codec).
Like that, if we want we can put this Codec private or directly implement the un/marshal directly in the functions etc...
There was a problem hiding this comment.
Still, I'm not convinced to that. I don't see pros of "hiding" that codec method.
Like that, if we want we can put this Codec private
So let's put the codec as private in the first place ;) But then you'll probably find that is not convenient to use :P
# Conflicts: # database/instance_db.go # database/instance_db_test.go # database/ownership_db.go # database/ownership_db_test.go # database/service_db.go # database/service_db_test.go # sdk/execution/execution_test.go # sdk/instance/backend.go # sdk/ownership/backend.go # sdk/service/backend.go
NicolasMahe
force-pushed
the
fix/codec-ante-handler
branch
from
4197ddb to
68aba4d
Compare
NicolasMahe
changed the title
dependency: #1462
By using the
auth.NewAnteHandlerwe let cosmos manage all the sequence numbers (so #1462 will work), but also tokens and fees for transactions (it seems but I didn't look at it a lot yet).By doing I realized that the ABCI messages should be pure data (as they are unmarshalled by the post validation step) so they cannot contain the codec as it was initially designed.
There is now a specific codec for each SDK that registers all the types needed for this SDK.
I also removed all the
mempool.ErrTxInCacheas this shouldn't happen anymore and if it happens that's an issue we need to fix and not hide with an "xxx already exists".