Skip to content
This repository was archived by the owner on Nov 17, 2025. It is now read-only.

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
@gumb0
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.

@chfast
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.

@chfast
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];
}

@axic
Copy link
Member

axic commented Sep 28, 2019

Added the size tests to Rust too.

@axic
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.

@chfast
Copy link
Member Author

chfast commented Nov 5, 2019

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

@atoulme atoulme closed this Jan 21, 2022
@chfast chfast reopened this Jan 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants