Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Lotus: fix eth address to f4 conversions #1030

Closed
Stebalien opened this issue Oct 26, 2022 · 5 comments
Closed

Lotus: fix eth address to f4 conversions #1030

Stebalien opened this issue Oct 26, 2022 · 5 comments
Assignees
Labels

Comments

@Stebalien
Copy link
Member

We have several places where we're directly constructing a "delegated" address from an eth address where we should be calling api.EthAddress(...).ToFilecoinAddress() (to correctly handle ID addresses).

@ychiaoli18
Copy link

ychiaoli18 commented Jan 6, 2023

@Stebalien after checking all the occurrences of address.NewDelegatedAddress, it seems that they all appear after hashing the pubkey to get the Ethereum address (with one exception in cli/chain.go), so the Ethereum address will not start with a [12]byte{0xff}. Therefore I don't think changing address.NewDelegatedAddress to ethtypes.EthAddress().ToFilecoinAddress() will solve any issue. Could you elaborate more about what you want to achieve?

@Stebalien
Copy link
Member Author

I filed this because I fixed a few cases long ago but didn't do an audit. If you've done a more thorough audit, we can consider this done.

Thanks!

@ychiaoli18
Copy link

@Stebalien I still pushed a PR that changed all the NewDelegatedAddress to ToFilecoinAddress(). It doesn't make much difference in functionality but creates f4 addresses more consistently. Do you think this should be merged?

@Stebalien
Copy link
Member Author

I think it makes sense anyways just for consistency. Otherwise, someone is likely to copy code and get it wrong.

@Stebalien Stebalien reopened this Jan 6, 2023
@raulk
Copy link
Member

raulk commented Jan 12, 2023

Merged in filecoin-project/lotus#9970.

@raulk raulk closed this as completed Jan 12, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants