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

[website/docs]: add page on EVM precompiles #944

Merged
merged 1 commit into from Nov 3, 2023

Conversation

jmcook1186
Copy link
Contributor

This PR adds a new page documenting the EVM precompiles included in Fe


### Parameters

- `input_buf`: a sequence of bytes to hash, `MemoryBuffer`
Copy link
Collaborator

Choose a reason for hiding this comment

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

The input argument at the callsite is buf; input_buf is the name of the value used internally in the function (argument labelling scheme was borrowed from swift, btw). The signature should probably be written sha2_256(buf: MemoryBuffer) -> u256.

(I'd argue that these single-argument functions should be defined as e.g. sha2_256(_ input_buf: MemoryBuffer) -> u256, so they can be called without a label on the arg.)

### Function signature

```fe,ignore,ignore
pub fn sha2_256(buf input_buf: MemoryBuffer) -> u256 {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since I'm nitpicking, the {} should be left off of the signature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like this?:

Suggested change
pub fn sha2_256(buf input_buf: MemoryBuffer) -> u256 {}
pub fn {} sha2_256(buf input_buf: MemoryBuffer) -> u256

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean "left off of" as in "not included". So, like this:
pub fn sha2_256(buf input_buf: MemoryBuffer) -> u256

Copy link
Collaborator

@sbillig sbillig left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for adding this.

We could borrow the descriptions of the functions and parameters from evm.codes (MIT licensed, should be fair game, right?) for a bit more detail, but I'd be happy to see this merged any time as it's obviously much better than what we have now.

@@ -81,6 +81,7 @@
* [Memory](spec/data_layout/memory/index.md)
* [Sequence types in memory](spec/data_layout/memory/sequence_types_in_memory.md)
* [Function calls](spec/function_calls.md)
* [Precompiles](spec/precompiles.md)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since precompiles are part of the std library, I believe this should sit outside of spec as spec is supposed to as close as possible a language specification (inspired by https://doc.rust-lang.org/stable/reference/introduction.html) and hence not cover std library stuff. I think that this should either become a sub of "Using Fe" or become a sub of a completely new item but I'm not sure what that would be (maybe "Standard Library"). The more I think about it, I think I like the idea of organizing this under a "Standard Library" section.

@cburgdorf cburgdorf merged commit d77c315 into ethereum:master Nov 3, 2023
7 checks passed
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.

None yet

3 participants