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.

Sign up for GitHub

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

[CORE-538] Address nit and fix Cosmos 0.50 post merge conflict #922

Merged
merged 2 commits into from
Jan 4, 2024
Merged

[CORE-538] Address nit and fix Cosmos 0.50 post merge conflict #922

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
37fc29e
[CORE-538] Address nit
lcwik Jan 4, 2024
e6dd665
Fix Cosmos 0.50 post merge conflict
lcwik Jan 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions protocol/app/process/other_msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ func DecodeOtherMsgsTx(decoder sdk.TxDecoder, txBytes []byte) (*OtherMsgsTx, err

// Validate returns an error if one of the underlying msgs fails `ValidateBasic`.
func (omt *OtherMsgsTx) Validate() error {
// Note that with Cosmos SDK 0.50.0 ValidateBasic has been removed from SDK message types,
// and it is recommended that users simulate the transaction instead to ensure that it could be
// processed.
for _, msg := range omt.msgs {
if m, ok := msg.(sdk.HasValidateBasic); ok {
if err := m.ValidateBasic(); err != nil {
Comment on lines 58 to 66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Validate method's comments indicate that ValidateBasic has been removed from SDK message types in Cosmos SDK 0.50.0, and transaction simulation is recommended instead. However, the code still checks for sdk.HasValidateBasic and calls ValidateBasic if available. This could lead to confusion and should be clarified or updated to reflect the new recommended practice.

- if m, ok := msg.(sdk.HasValidateBasic); ok {
-     if err := m.ValidateBasic(); err != nil {
+ // if m, ok := msg.(sdk.HasValidateBasic); ok {
+ //     if err := m.ValidateBasic(); err != nil {
+ //         return getValidateBasicError(msg, err)
+ //     }
+ // }
+ // TODO: Implement transaction simulation as per Cosmos SDK 0.50.0 recommendations
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// Validate returns an error if one of the underlying msgs fails `ValidateBasic`.
func (omt *OtherMsgsTx) Validate() error {
// Note that with Cosmos SDK 0.50.0 ValidateBasic has been removed from SDK message types,
// and it is recommended that users simulate the transaction instead to ensure that it could be
// processed.
for _, msg := range omt.msgs {
if m, ok := msg.(sdk.HasValidateBasic); ok {
if err := m.ValidateBasic(); err != nil {
// Validate returns an error if one of the underlying msgs fails `ValidateBasic`.
func (omt *OtherMsgsTx) Validate() error {
// Note that with Cosmos SDK 0.50.0 ValidateBasic has been removed from SDK message types,
// and it is recommended that users simulate the transaction instead to ensure that it could be
// processed.
for _, msg := range omt.msgs {
// if m, ok := msg.(sdk.HasValidateBasic); ok {
// if err := m.ValidateBasic(); err != nil {
// return getValidateBasicError(msg, err)
// }
// }
// TODO: Implement transaction simulation as per Cosmos SDK 0.50.0 recommendations

Expand Down
Loading