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

cpp: Add Host interface for the Host side #226

Merged
merged 5 commits into from
Apr 2, 2019
Merged

Conversation

chfast
Copy link
Member

@chfast chfast commented Mar 26, 2019

This adds the Host part of the C++ API.

The plan was different, to keep it physically separated, but I realized both Host and VM sides of the API implement the same interface... HostInterface.

This introduces some naming issues. We have

  • HostInterface as pure C++ interface
  • HostContext as wrapper for the host context pointer to be used by VMs, implements HostInterface.
  • Host - the abstract class for Host implementations, implements HostInterface.

Suggestions for better names welcome.

Effectively closes #189.

The only tunes left for C++ are aliases like evmc::address = evmc_address, merging helpers.hpp, maybe add better types for evmc_address, evmc_uint256be, etc.

@chfast chfast force-pushed the cpp-host-interface branch 3 times, most recently from 2cbd68b to 886cecc Compare March 28, 2019 16:56
@chfast chfast marked this pull request as ready for review March 28, 2019 17:01
@chfast chfast requested review from axic and gumb0 March 28, 2019 17:05
Copy link
Member

@gumb0 gumb0 left a comment

Choose a reason for hiding this comment

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

Looks good to me

account_exists, get_storage, set_storage, get_balance, get_code_size, get_code_hash,
copy_code, selfdestruct, call, get_tx_context, get_block_hash, emit_log,
public:
bool account_exists(const evmc_address& addr) noexcept override
Copy link
Member

Choose a reason for hiding this comment

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

could be final

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I noticed. Decided to go override because more common. I tend to not to use less common C++ features in examples. Although, in this case it might be good suggestion to use final here.

/// Wrapper around EVMC host context / host interface.
class host
///
/// To be used by VM implementations as better alternative than using ::evmc_context directly.
Copy link
Member

Choose a reason for hiding this comment

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

alternative to

{
context->host->emit_log(context, &address, data, data_size, topics, topics_count);
}
};

/// Abstract class to be used by Host implementations. Hides ::evmc_context vtable.
Copy link
Member

Choose a reason for hiding this comment

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

Documentation here could be better. evmc_context doesn't have vtable.

Better explicitly point out, that this class should be derived when implementing Host

@chfast chfast merged commit ea4bf5c into master Apr 2, 2019
@chfast chfast deleted the cpp-host-interface branch April 2, 2019 10:13
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.

Create C++ bindings for writing VMs
2 participants