Separate API Tests for the VM #310

holgerd77 · 2018-07-13T08:49:53Z

Introduction / Situation

In the current tests setup only the official Ethereum state and blockchain tests are run to test the VM functionality.

This gives a good impression of the current state of compatibility towards the latest HF changes but leaves the outer API of the library more-or-less untested, see the red files in one of the latest coverage reports.

This is simply not sufficient, since this currently means that various possible invocations of the API (instantiation with or without blockchain,...) as well as larger parts of the non-VM-executing code parts remain completely untested, see e.g. the poor coverage for the runBlock.js file (< 20%, sigh).

Task Description

This can be tackled by adding a second test suite and setup, resembling more traditional tests like e.g. in the block library (super-random example, don't go along too closely) and testing the different instantiation and execution paths for the VM.

Like always with tests even a very basic test setup with just a handful of tests would already make a huge difference and prevent issues like #303 where functionality completely broke and no one noticed.

On a side note: Beside improve test coverage and library quality this will also have the benefit of giving a comprise and up-to-date overview on library usage and instantiation, since our examples have a tendency to always be out-of-date.

Goals

A suite of initially maybe 15-20 additional tests - e.g. in tests/api/ should be developed which tests:

At least 2-3 additional instantiation paths of the VM in index.js
Larger parts of runBlockchain.js
Larger parts of runBlock.js
1-2 new instantiation paths in runTx.js
Additional parts of bloom.js, fakeBlockChain.js, runJit.js (scope to be determined)
StateManager is under refactoring so not too much emphasis on this, maybe 1-2 tests to start would be nevertheless good

Generally the setup of a high-quality test structure with non-code repetition, utility functions and eventually good structured test data has precedence over the amount and extent of tests or coverage.

Skills

For taking on this issue a solid understanding of the internal working of the Ethereum VM and some broader picture of the execution process within a blockchain environment is needed.

Generally this should be a really rewarding task since this will frankly lead to a deeper understanding of the various parts of the VM on tackling.

The text was updated successfully, but these errors were encountered:

gitcoinbot · 2018-07-13T10:11:18Z

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 440.0 DAI (440.0 USD @ $1.0/DAI) attached to it.

If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
Questions? Checkout Gitcoin Help or the Gitcoin Slack
$22,730.94 more funded OSS Work available on the Gitcoin Issue Explorer

gitcoinbot · 2018-07-13T10:20:54Z

Issue Status: 1. Open 2. Cancelled

The funding of 440.0 DAI (440.0 USD @ $1.0/DAI) attached to this issue has been cancelled by the bounty submitter

Questions? Checkout Gitcoin Help or the Gitcoin Slack
$22,290.94 more funded OSS Work available on the Gitcoin Issue Explorer

gitcoinbot · 2018-07-13T11:23:17Z

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 340.0 DAI (340.0 USD @ $1.0/DAI) attached to it.

If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
Questions? Checkout Gitcoin Help or the Gitcoin Slack
$22,780.94 more funded OSS Work available on the Gitcoin Issue Explorer

s1na · 2018-07-13T12:37:02Z

@holgerd77 I would be interested in tackling this, specially because I'm hoping to work on ewasm later on and this task would increase my understanding of the vm by quite a bit. However, I'm not deeply familiar with inner workings of the vm as of yet. Is it okay if I give it a try? or this is a high-priority task which should be done asap?

holgerd77 · 2018-07-13T18:52:02Z

@s1na It is that sense "high-priority" that it should meet certain quality standards at the end, this is not super time-critical though. If you have got some experience in writing tests and think that you are close enough with your understanding of the broader ecosystem that encircling this specific field is not too hard, go ahead and give it a try, looking forward to it! 😄

Will to some extend be there for some help if you are in need, do have to level down work in the coming weeks though due to personal reasons.

s1na · 2018-07-13T19:18:22Z

@holgerd77 Okay, I will then try to create a PR early, so that with your helpful comments in the review I can hopefully satisfy the standards :)

gitcoinbot · 2018-07-13T19:23:53Z

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 4 months, 4 weeks ago.
Please review their action plans below:

1) s1na has been approved to start work.

I will have a more detailed look around the code and the existing tests, and then start to write tests for the low coverage parts that were mentioned in the issue.

Learn more on the Gitcoin Issue Details page.

gitcoinbot · 2018-07-17T16:15:38Z

@s1na Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

warning (3 days)
escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

s1na · 2018-07-17T16:28:38Z

Hi, I'm going through the relevant parts of the yellow paper and the source code to solidify my understanding, and early next week I'll create the initial PR (I won't be able to work for the next 3 days as I'm attending the dappcon).

vs77bb · 2018-07-20T06:43:04Z

Thanks for the heads up @s1na - have fun at dappcon! Say hi to @ceresstation / @mbeacom if you see them 😄

holgerd77 · 2018-07-20T06:57:20Z

@s1na @vs77bb Ah, you're already in social exchange, great 😄! Enjoy dappcon, can't be there unfortunately!

mkosowsk · 2018-07-26T20:14:03Z

Hi @s1na, just a friendly check-in here 😀 have you had a chance to take a look at this issue? 👍

s1na · 2018-07-26T20:29:40Z

Hey @mkosowsk :) I'm tackling the issue here.

mkosowsk · 2018-07-26T20:32:46Z

@s1na thanks! Will keep my eyes peeled, just wanted to make sure that you got paid out as soon as your work is accepted! 👍👍👍

Thoughtscript · 2018-08-04T23:35:11Z

Hi! Just checking to see if there's any work on this that still needs doing! Thanks!

gitcoinbot · 2018-08-07T16:15:15Z

@s1na Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

warning (3 days)
escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

s1na · 2018-08-07T19:47:11Z

@gitcoinbot Hi :) Yes, I'm still working on the issue.

gitcoinbot · 2018-08-16T16:15:13Z

@s1na Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

warning (3 days)
escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

s1na · 2018-08-16T20:52:05Z

@gitcoinbot yep :)

gitcoinbot · 2018-08-25T16:15:08Z

@s1na Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

warning (3 days)
escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

s1na · 2018-08-26T14:35:13Z

Hey @gitcoinbot, yes I'm still working on the issue :)

vs77bb · 2018-09-25T03:55:05Z

Hi @s1na I know you've been moving this along. Mind letting us know what's left in order for the bounty to be claimed? cc @holgerd77 for his context, as well 👍

s1na · 2018-09-25T07:31:53Z

Hey @vs77bb, thanks for following the progress :) to my knowledge, I should extract 2 more PRs out of #315 for review, in addition to #352 which is close to being merged I think.

holgerd77 · 2018-09-25T11:55:22Z

Hi @vs77bb, thanks for following up on this. I was somewhat slow with reviewing in the last weeks due to family reasons + Sina did a lot more than what I expected to be done for this bounty. 😄

Nevertheless I also had the thought a couple of days ago, that we should really come to a close on this one, maybe we can start a follow-up bounty with some more tests, but that's on another plate.

I will have somewhat more time in the coming 2-3 weeks and will also prioritize reviewing the last 2 PRs. Thanks for making this possible, this work is really substantially contributing to sustain and improve the quality of our VM code!

holgerd77 · 2018-10-02T11:15:39Z

Hi @vs77bb Vivek, this is now completed and @s1na did a great work on this, can we have a bounty payout here? Is there anything I need to do for this to be processed?

spm32 · 2018-10-02T15:31:25Z

Hey @s1na can you submit work on the associated Gitcoin bounty? I'll pay you out as soon as that's done, awesome work!

gitcoinbot · 2018-10-02T16:32:34Z

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 340.0 DAI (340.0 USD @ $1.0/DAI) has been submitted by:

@s1na

@ceresstation please take a look at the submitted work:

(Link Not Provided) by @s1na

Learn more on the Gitcoin Issue Details page
Want to chip in? Add your own contribution here.
Questions? Checkout Gitcoin Help or the Gitcoin Slack
$25,872.10 more funded OSS Work available on the Gitcoin Issue Explorer

gitcoinbot · 2018-10-02T17:55:02Z

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 340.0 DAI (340.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @s1na.

Learn more on the Gitcoin Issue Details page
Questions? Checkout Gitcoin Help or the Gitcoin Slack
$25,632.10 more funded OSS Work available on the Gitcoin Issue Explorer

holgerd77 added type: feature help wanted labels Jul 13, 2018

holgerd77 mentioned this issue Jul 20, 2018

Use 11-bit mask in Bloom.check #311

Merged

s1na mentioned this issue Jul 22, 2018

Add Separate API Tests for the VM #315

Closed

s1na mentioned this issue Jul 31, 2018

Fix error handling in fakeBlockChain #320

Merged

holgerd77 added PR state: WIP type: tests and removed help wanted type: feature labels Sep 20, 2018

holgerd77 closed this as completed Oct 2, 2018

evertonfraga added the package: vm label Feb 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate API Tests for the VM #310

Separate API Tests for the VM #310

holgerd77 commented Jul 13, 2018 •

edited

Loading

gitcoinbot commented Jul 13, 2018

gitcoinbot commented Jul 13, 2018

gitcoinbot commented Jul 13, 2018

s1na commented Jul 13, 2018

holgerd77 commented Jul 13, 2018 •

edited

Loading

s1na commented Jul 13, 2018

gitcoinbot commented Jul 13, 2018 •

edited

Loading

gitcoinbot commented Jul 17, 2018

s1na commented Jul 17, 2018

vs77bb commented Jul 20, 2018 •

edited

Loading

holgerd77 commented Jul 20, 2018

mkosowsk commented Jul 26, 2018

s1na commented Jul 26, 2018

mkosowsk commented Jul 26, 2018

Thoughtscript commented Aug 4, 2018

gitcoinbot commented Aug 7, 2018

s1na commented Aug 7, 2018

gitcoinbot commented Aug 16, 2018

s1na commented Aug 16, 2018

gitcoinbot commented Aug 25, 2018

s1na commented Aug 26, 2018

vs77bb commented Sep 25, 2018

s1na commented Sep 25, 2018

holgerd77 commented Sep 25, 2018

holgerd77 commented Oct 2, 2018

spm32 commented Oct 2, 2018 •

edited

Loading

gitcoinbot commented Oct 2, 2018

gitcoinbot commented Oct 2, 2018

Separate API Tests for the VM #310

Separate API Tests for the VM #310

Comments

holgerd77 commented Jul 13, 2018 • edited Loading

gitcoinbot commented Jul 13, 2018

gitcoinbot commented Jul 13, 2018

gitcoinbot commented Jul 13, 2018

s1na commented Jul 13, 2018

holgerd77 commented Jul 13, 2018 • edited Loading

s1na commented Jul 13, 2018

gitcoinbot commented Jul 13, 2018 • edited Loading

gitcoinbot commented Jul 17, 2018

s1na commented Jul 17, 2018

vs77bb commented Jul 20, 2018 • edited Loading

holgerd77 commented Jul 20, 2018

mkosowsk commented Jul 26, 2018

s1na commented Jul 26, 2018

mkosowsk commented Jul 26, 2018

Thoughtscript commented Aug 4, 2018

gitcoinbot commented Aug 7, 2018

s1na commented Aug 7, 2018

gitcoinbot commented Aug 16, 2018

s1na commented Aug 16, 2018

gitcoinbot commented Aug 25, 2018

s1na commented Aug 26, 2018

vs77bb commented Sep 25, 2018

s1na commented Sep 25, 2018

holgerd77 commented Sep 25, 2018

holgerd77 commented Oct 2, 2018

spm32 commented Oct 2, 2018 • edited Loading

gitcoinbot commented Oct 2, 2018

gitcoinbot commented Oct 2, 2018

holgerd77 commented Jul 13, 2018 •

edited

Loading

holgerd77 commented Jul 13, 2018 •

edited

Loading

gitcoinbot commented Jul 13, 2018 •

edited

Loading

vs77bb commented Jul 20, 2018 •

edited

Loading

spm32 commented Oct 2, 2018 •

edited

Loading