Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings
This repository was archived by the owner on Nov 17, 2025. It is now read-only.

Comments

Scratchpad#434

Draft
chfast wants to merge 5 commits intomaster from
scratchpad
Draft

Scratchpad #434
chfast wants to merge 5 commits intomaster from
scratchpad

Conversation

@chfast
Copy link
Member

@chfast chfast commented Sep 26, 2019

The idea behind this is the following:

  1. Scratchpad is a small buffer attached to evmc_result. That's there is up the the creator of the evmc_result object.
  2. The VM can copy small EVM outputs there (no dynamic allocation). Usually solidity functions return nothing or 32 bytes.
  3. This is also the way to deliver "create address" from Client to VM. Union is used because the "create address" and output is never needed at the same time.

@chfast chfast requested review from axic and gumb0 September 26, 2019 15:48
Copy link
Member

gumb0 commented Sep 26, 2019

I think it certainly looks simpler without additional free functions

* Also extends the size of the evmc_result to 64 bytes (full cache line).
*/
uint8_t padding[4];
union evmc_result_scratchpad scratchpad;
Copy link
Member

Choose a reason for hiding this comment

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

I think for a 4 byte saving this complicates matters too much for bindings, such as go or rust.

Copy link
Member Author

chfast commented Sep 27, 2019

Risks:

  1. In theory, union memory layout is not binary compatible across different compilers / compilers versions.
  2. Not everything FFI/bindings support unions. E.g. Go will represent this union as bytes.
  3. Using the same memory space for different purposes.

A more conservative variant is to place the scratchpad after create_address.

Copy link
Member Author

chfast commented Sep 28, 2019

Update version: create_address separated from the scratchpad.

Still union is used - this is the easiest way to make sure the scratchpad is aligned memory (fix needed for 32-bit arch). However, using unions still might be not ideal for bindings.

Alternatively, we can define scratchpad as uint8_t scratchpad[32 + 4] and then add helpers like:

uint64_t* get_scratchpad_as_words(result*)
{
 (uint64_t*)&result->scratchpad[4];
}

Copy link
Member

axic commented Sep 28, 2019

Added the size tests to Rust too.

Copy link
Member

axic commented Nov 5, 2019

@chfast how about this PR? The current version seems to be good, at least from the C and Rust perspectives.

Copy link
Member Author

chfast commented Nov 5, 2019

Not now. I have some other ideas in this subject.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@gumb0 gumb0 Awaiting requested review from gumb0

1 more reviewer

@axic axic axic left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /