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

lotus-fountain: make compatible with 0x addresses #10560 #10784

Merged
merged 2 commits into from
May 12, 2023

Conversation

maciejwitowski
Copy link
Contributor

Related Issues

Closes #10560

Proposed Changes

Check if address starts with Ox. If so, parse it as Eth address. Otherwise, use the previous logic.

}
to = filecoinAddress
} else {
filecoinAddress, err := address.NewFromString(r.FormValue("address"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
filecoinAddress, err := address.NewFromString(r.FormValue("address"))
filecoinAddress, err := address.NewFromString(addressInput)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot, will merge into the commit.

http.Error(w, err.Error(), http.StatusBadRequest)
return
}
to = filecoinAddress
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it ok to skip the check for empty address with checking filecoinAddress == address.Undef like when its not a 0x address? Can we make the checks for both scenarios the same and maybe refactor the code a bit to use the same error checking code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToFilecoinAddress can return address.Undef but in such case it also returns error so it's not strictly needed because the error is checked here. The point about having the same error handling is legit, will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fridrik01 Is this the way to go in Go 6c25f21 ? 🙂

@@ -193,12 +195,30 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

to, err := address.NewFromString(r.FormValue("address"))
if err != nil {
var to address.Address
Copy link
Contributor

Choose a reason for hiding this comment

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

We're not setting this anywhere, and using for the send destination, probably want to remove this var, and swap uses of to to filecoinAddress

Copy link
Contributor

@fridrik01 fridrik01 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@fridrik01 fridrik01 left a comment

Choose a reason for hiding this comment

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

LGTM

@arajasek arajasek merged commit b3d0b18 into master May 12, 2023
93 checks passed
@arajasek arajasek deleted the maciej/fountain branch May 12, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lotus-fountain: make compatible with 0x addresses
6 participants