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

Add vm test for execute #94

Merged
merged 2 commits into from
Aug 28, 2018
Merged

Add vm test for execute #94

merged 2 commits into from
Aug 28, 2018

Conversation

axic
Copy link
Member

@axic axic commented Aug 27, 2018

Unfortunately need to adjust all the test VMs.

@axic axic added this to In progress in v5.2 via automation Aug 27, 2018
@axic
Copy link
Member Author

axic commented Aug 27, 2018

Oh, well great, this uses ~/install/bin/evmc-vmtester ~/install/lib/libevmc-examplevm.so which is passing the tests.

@@ -31,6 +31,25 @@ TEST_F(evmc_vm_test, abi_version_match)
ASSERT_EQ(vm->abi_version, EVMC_ABI_VERSION);
}

TEST_F(evmc_vm_test, execute)
{
struct evmc_context context{};
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to populate this with callbacks.

@chfast
Copy link
Collaborator

chfast commented Aug 28, 2018

I'm rebasing this...

@axic
Copy link
Member Author

axic commented Aug 28, 2018

It is broken now:

Build-agent version 0.1.437-d180d4d7 (2018-08-22T16:07:19+0000)
Starting container ethereum/cpp-build-env
  image is cached as ethereum/cpp-build-env, but refreshing...

Error response from daemon: manifest for ethereum/cpp-build-env:latest not found

(void)topics_count;
}

extern const struct evmc_context_fn_table mock_context_fn_table = {
Copy link
Member Author

Choose a reason for hiding this comment

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

In the future we could extend this to have a factory which instantiates a context with certain values.

@axic axic changed the title [WIP] Add vm test for execute Add vm test for execute Aug 28, 2018
@axic axic force-pushed the vm-test-execute branch 2 times, most recently from c558940 to 35da9ae Compare August 28, 2018 17:30
@axic
Copy link
Member Author

axic commented Aug 28, 2018

@chfast this is ready now

Copy link
Collaborator

@chfast chfast left a comment

Choose a reason for hiding this comment

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

Very nice.

I'm just thinking about renaming "context" to "host"...

#include <stdlib.h>
#include <string.h>

struct evmc_uint256be balance(struct evmc_context* context, const struct evmc_address* address)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need struct prefix in C++.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a copy from examples/capi.c. Should we keep it as C (with a .c extension) or go full in on C++?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think C++ will be easier.

struct evmc_message msg
{
};
std::vector<uint8_t> code({0xfe, 0x00});
Copy link
Collaborator

Choose a reason for hiding this comment

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

std::array would go here. Or even a raw array + std::begin and std::size.

@axic
Copy link
Member Author

axic commented Aug 28, 2018

I'm just thinking about renaming "context" to "host"...

Sounds good. Obviously that is a separate breaking change.

@axic axic force-pushed the vm-test-execute branch 3 times, most recently from 80d55bc to 5516456 Compare August 28, 2018 18:34
@axic
Copy link
Member Author

axic commented Aug 28, 2018

@chfast updated

@chfast
Copy link
Collaborator

chfast commented Aug 28, 2018

I think mock_context.c should actually replace capi.c as a host implementation example. But this can be done later.

v5.2 automation moved this from In progress to Reviewer approved Aug 28, 2018
@chfast chfast merged commit 872ded2 into master Aug 28, 2018
v5.2 automation moved this from Reviewer approved to Done Aug 28, 2018
@chfast chfast deleted the vm-test-execute branch August 28, 2018 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v5.2
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants