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

Add a chain ID to the signature of FVM messages #339

Closed
aakoshh opened this issue May 19, 2023 · 1 comment · Fixed by consensus-shipyard/fendermint#115
Closed

Add a chain ID to the signature of FVM messages #339

aakoshh opened this issue May 19, 2023 · 1 comment · Fixed by consensus-shipyard/fendermint#115

Comments

@aakoshh
Copy link
Contributor

aakoshh commented May 19, 2023

Originated from this Slack discussion.

FEVM messages are chain-id separated today. FVM/native-Filecoin messages are not. This is an issue if you expose non-FEVM user addresses in IPC.

This is the FIP explaining how the FEVM chain ID separation works: https://github.com/filecoin-project/FIPs/blob/3a4ba11c41e8a8e641d27b8999dab3a774b2751e/FIPS/fip-0055.md#delegated-signature-type

And this one never got implemented: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0039.md

Fendermint currently doesn't have a concept of delegated addresses. Forest has an is_delegated method but at the time when I looked it didn't use it for anything, so I didn't investigate and only used the SignatureType from fvm_shared which doesn't have the concept of a delegated type, so I did the same check as Forest, which includes no domain separation (ie. no chain ID). Forest uses a shim for the signature, which is where they added this new type, no doubt for future use (I even saw the Slack convo where they asked why it's not in the FVM).

As I understand FIP-55 has to be implemented by the node for FEVM type messages, and to protect against replay attacks across subnets, it would be enough to hash the chain ID (which the node knows) together with the CID of the message, without adding it to the message. A new --chain-id option has to be added to all the CLI commands, though, and the MessageFactory in the RPC library.

@aakoshh
Copy link
Contributor Author

aakoshh commented May 22, 2023

https://gist.github.com/rekmarks/a47bd5f2525936c4b8eee31a16345553 explains the maximum safe integer MetaMask uses for chain ID.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants