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

Initial draft of FRC for calling convention #399

Merged
merged 3 commits into from
Jul 26, 2022

Conversation

anorth
Copy link
Member

@anorth anorth commented Jul 7, 2022

An actor calling convention that derives method numbers by hashing method names.

This proposal describes a convention for the labelling of methods across actors which will support the development of standardised APIs, e.g. a token API. The convention maps human-friendly method names to method numbers by hashing the method name. This supports the definition of multiple standardized APIs with minimal potential for collision on method numbers.

See #382 for discussion.
PTAL @Kubuxu @raulk @Stebalien

@Tom-OriginStorage
Copy link

Tom-OriginStorage commented Jul 7, 2022

(Edited by @anorth: discussion moved to #382 (reply in thread))

let domain_separation_tag = "1|" // Note `|` is illegal in method names
let mut digest Vec<u8>= blake2b(domain_separation_tag + method_name)
while !digest.is_empty() {
let method_id = u32::from_be(digest[0..4])
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming we are using big-endian to be consistent with existing protocols.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's "network" byte order and also the order used by CBOR.

Co-authored-by: Jakub Sztandera <kubuxu@protocol.ai>
@jennijuju
Copy link
Member

jennijuju commented Jul 26, 2022

The draft lgtm, more discussion may take place in #382 and to be
addressed in follow-up PRs - 'd like to assign this FRC as 0042, and will be happy to approve it once the assigned number is getting updated in the proposal & filename.

@kaitlin-beegle
Copy link
Contributor

Agreed! Thanks for reviewing.

Not to state the obvious, but I think we should recognize a policy wherein any FIP editor who authors a FIP/FRC cannot also be the primary reviewer for their work. Glad you were able to take a look at this :)

@kaitlin-beegle kaitlin-beegle merged commit f41f6cb into master Jul 26, 2022
@kaitlin-beegle kaitlin-beegle deleted the anorth/calling-convention branch July 26, 2022 07:22
@jennijuju
Copy link
Member

@kaitlin-beegle i think we will have to revert the PR, or create a new pr as FIP number was not updated in the proposal yet


Note that removing both those differences would *not* automatically make this scheme compatible with the Solidity ABI,
since method parameter types have fundamentally different schemas between the two environments.
A future FVM/EVM embedding will need to translate method selectors along with the rest of the EVM ABI.
Copy link
Member

Choose a reason for hiding this comment

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

I doubt we will have logic to translate method selectors; that would rely on a number of assumptions that I don't feel good encoding in the protocol (even if in actor land). Instead, Ethereum CALLs would set a fixed dynamic method number and rely on the call convention in parameters to perform dispatch.

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.

6 participants