fix npe when memo is not empty #77
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolaslara
approved these changes
Aug 3, 2023
mergify bot
pushed a commit
that referenced
this pull request
Aug 6, 2023
* fix npe when memo is not empty * test: add e2e tests for pfm --------- Co-authored-by: Justin Tieri <37750742+jtieri@users.noreply.github.com> Co-authored-by: jtieri <justin@thetieris.com> (cherry picked from commit 7c8f814)
mergify bot
pushed a commit
that referenced
this pull request
Aug 6, 2023
* fix npe when memo is not empty * test: add e2e tests for pfm --------- Co-authored-by: Justin Tieri <37750742+jtieri@users.noreply.github.com> Co-authored-by: jtieri <justin@thetieris.com> (cherry picked from commit 7c8f814)
This was referenced Aug 6, 2023
mergify bot
pushed a commit
that referenced
this pull request
Aug 6, 2023
* fix npe when memo is not empty * test: add e2e tests for pfm --------- Co-authored-by: Justin Tieri <37750742+jtieri@users.noreply.github.com> Co-authored-by: jtieri <justin@thetieris.com> (cherry picked from commit 7c8f814)
agouin
added a commit
that referenced
this pull request
Aug 7, 2023
* fix npe when memo is not empty * test: add e2e tests for pfm --------- Co-authored-by: Justin Tieri <37750742+jtieri@users.noreply.github.com> Co-authored-by: jtieri <justin@thetieris.com> (cherry picked from commit 7c8f814) Co-authored-by: Andrew Gouin <andrew@gouin.io>
agouin
added a commit
that referenced
this pull request
Aug 7, 2023
* fix npe when memo is not empty * test: add e2e tests for pfm --------- Co-authored-by: Justin Tieri <37750742+jtieri@users.noreply.github.com> Co-authored-by: jtieri <justin@thetieris.com> (cherry picked from commit 7c8f814) Co-authored-by: Andrew Gouin <andrew@gouin.io>
agouin
added a commit
that referenced
this pull request
Aug 7, 2023
* fix npe when memo is not empty * test: add e2e tests for pfm --------- Co-authored-by: Justin Tieri <37750742+jtieri@users.noreply.github.com> Co-authored-by: jtieri <justin@thetieris.com> (cherry picked from commit 7c8f814) Co-authored-by: Andrew Gouin <andrew@gouin.io>
nicolaslara
reviewed
Aug 10, 2023
| @@ -370,6 +370,7 @@ func (k *Keeper) RetryTimeout( | |||
| } | |||
|
|
|||
| if data.Memo != "" { | |||
| metadata.Next = &types.JSONObject{} | |||
| if err := json.Unmarshal([]byte(data.Memo), metadata.Next); err != nil { | |||
| return fmt.Errorf("error unmarshaling memo json: %w", err) | |||
There was a problem hiding this comment.
I'm taking a second look at this in the context of osmosis-labs/osmosis#5969. It seems like some packets are returning this error. I think that error should never happen: if the unmarshaling fails we should just return a json object that is a string.
This should in theory be what's happening inside JSONObject.UnmarshalJSON(), but I think it needs to be reviewed as it only accepts json objects or true/false and not arbitrary strings
6 tasks
xlab
pushed a commit
to persistenceOne/ibc-apps
that referenced
this pull request
Jan 9, 2024
* fix npe when memo is not empty * test: add e2e tests for pfm --------- Co-authored-by: Justin Tieri <37750742+jtieri@users.noreply.github.com> Co-authored-by: jtieri <justin@thetieris.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When next is populated in the forward metadata, a timeout on the forward would cause the timeout to fail to process due to this nil pointer exception.