Where communities thrive

Join over 1.5M+ people
Join over 100K+ communities
Free without limits
Create your own community

Explore more communities

ethereum/py-evm

People

Repo info

Activity

May 29 21:07

carver edited #1943
May 29 21:07

carver edited #1943
May 29 21:06

carver edited #1943
May 29 21:05

carver review_requested #1943
May 29 21:04

carver edited #1943
May 29 21:02

carver opened #1943
May 29 20:23

veox edited #1942
May 29 20:22

veox edited #1942
May 29 20:19

veox edited #1940
May 29 20:19

veox edited #1940
May 29 20:18

veox opened #1942
May 29 20:05

veox edited #1940
May 29 20:05

veox synchronize #1940
May 29 19:57

veox commented #1940
May 29 19:51

veox edited #1940
May 29 19:51

veox edited #1940
May 29 19:50

veox synchronize #1940
May 29 18:16

carver commented #1940
May 29 18:16

carver commented #1940
May 29 18:14

carver commented #1940

Noel Maersk

@veox

... but it seems that geth is not as stringent as py-evm on when this should happen. We only do it if computation.is_origin_computation.

Noel Maersk

@veox

Not 100% sure this is the culprit, though - still looking...

Noel Maersk

@veox

Yup, that's it. yield THREE (unconditional) passes the test. So, just need to cherry-pick the conditions, so that other tests don't all fail...

Noel Maersk

@veox

OK, I can't figure out how to do this without a double-pass through the nested computations, or ugly hacks - not today, at least, and got to run anyway.

I liked it more when aleth was filling the tests. :/

Summary of noise: https://github.com/ethereum/py-evm/issues/1857#issuecomment-538460472

Tan

@tanmaster

hi, i kinda feel stupid for asking this, but is there a way to set the code of an address with vm.state.setCode() and then persist it without getting a validation error when mining the block?

Tan

@tanmaster

nevermind, i solved it: i extended the IstanbulVM class and the MiningChain class, overrid mine_block in both and commented out the lines in which the validation happens.

Voith Mascarenhas

@voith

@veox nice work! With my current knowledge it would have taken me ages to figure that out 😅

Noel Maersk

@veox

veox/py-evm@6c6db73 - supposed fix (for 4 of 6 failures).

Noel Maersk

@veox

I'm relatively certain that the branch will now pass all the tests (i.e. fixed the remaining 2).

Noel Maersk

@veox

@voith and all: should I submit the fixes as a PR for PR #1852, or roll a new PR including everything?..

Y'all' be at devcon, so there won't be anyone to review/merge it anyway...

Voith Mascarenhas

@voith

@veox do anything that is convenient for you. I have no problem closing the PR that I've opened.

Jason Carver

@carver

Yeah, I'm not checking github notifications regularly, but if you @ mention me in here, I'll get to it soon-ish @veox

Chris Calderon

@SerpentChris

Tried install py-evm on PPC64 Linux today and it failed :( . One of the dependencies, blake2b-py fails to build because it's dependency, marturin, fails to build because it's rust dependency ring v0.16.9, fails to build.

Chris Calderon

@SerpentChris

Is there some reason blake2b-py is used instead of the blake functions in hashlib? They are standard starting with python version 3.6

Chris Calderon

@SerpentChris

OH, it looks like you switched to this module because it is "560x" faster about 21 days ago.

Chris Calderon

@SerpentChris

For now I think I can just checkout the commit before the merge. Unrelatedly, there is another problem that stops me from being able to easily use py-evm on PPC64 big endian Linux, which is that the pyethash version this depend on seems to have a problem compiling on big endian platforms. This issue was fixed in the pyethash repo but I don't think that fix was put onto pypi, so the version pip tries to install fails.

Chris Calderon

@SerpentChris

I've made an issue with ring, so maybe they can fix the build failure.

Jason Carver

@carver

For now I think I can just checkout the commit before the merge.

Yup, this is a reasonable solution for now. It is possible to make the blake2b-py install optional and maintain the pure python implementation as a backup. It's probably not top priority for us, but if you open an issue or want to work on it yourself I'm happy to guide on the necessary steps.

the pyethash version this depend on seems to have a problem compiling on big endian platforms. This issue was fixed in the pyethash repo but I don't think that fix was put onto pypi, so the version pip tries to install fails.

Can you link to the issue or the fix in the repo? I can push out a release, just want to make sure I understand the change you need so I can verify it's working, before release.

@SerpentChris

Chris Calderon

@SerpentChris

ethereum/ethash@4c0822a

On my big endian system, when I recently tried to install ethash via pip it complained about the issue that was fixed by this commit.

Chris Calderon

@SerpentChris

I think I found the issue? PyPI has an ehtash package and a pyethash package. The ethash package installs fine but py-evm tries to install the pyethash package which is broken.

Honestly I'm a bit confused here. These packages are all on PyPI: ethash, pyethash, and eth-hash.

Bryant Eisenbach

@fubuloubu

was there any change in how selfdestruct is being handled for Istanbul?

IIRC there was not, but we are experiencing some issues: ethereum/vyper#1657

Voith Mascarenhas

@voith

@fubuloubu I just had a look at the test failures and it seems like the transactions need more gas.

Bryant Eisenbach

@fubuloubu

The gas cost of SELFDESTRUCT wasn't changed though, was it?

Voith Mascarenhas

@voith

I just went through the Istanbul meta EIP and it doesn’t seem like the gas cost of SELFDESTRUCT was changed.

Bryant Eisenbach

@fubuloubu

yup

I forgot that this has nothing to do with Istanbul because the issue gets introduced when we use eth-tester v0.2b1+

(which uses py-evm 0.2.0a43+ under the hood)

Istanbul was only introduced in eth-tester v0.2b3 I believe

Voith Mascarenhas

@voith

Hmmm, yeah. But istanbul could break some tests, specially ethereum/EIPs#2200

Bryant Eisenbach

@fubuloubu

I replaced in our test cases SELFDESTRUCT with a simple send that replicates the same result, and it passes

Voith Mascarenhas

@voith

:+1:

Bryant Eisenbach

@fubuloubu

btw @carver the CHANGELOG for eth-tester still says v0.2.0b3 is unreleased

Noel Maersk

@veox

We should probably change 7_000_000 to 5_000_000 (same as Constantinople/Petersburg) on this line, if there is to be no difficulty bomb reset for Istanbul. (And looks like there won't be.) (EDIT: Or use the petersburg function altogether.)

I've done some slackjob calculations, and (given no changes in net-wide hashrate, a big given) the bomb won't make 10% of the difficulty for ~500 more days.

A tad too optimistic for my taste, since unexpected delays are always possible (as Constantinople/Petersburg showed), but I'll refrain from suggesting in ACD a "reset" should be done preemptively.

Christoph Burgdorf

@cburgdorf

Just merged Clique Consensus PR ethereum/py-evm#1855 Trinity side should follow soon.

Noel Maersk

@veox

QQ: Does newsfragments support multiple lines?..

Noel Maersk

@veox

As in, there are two noteworthy changes in PR #1878, that should both be listed...

OK, towncrier --draft is my friend. ^_^

Noel Maersk

@veox

Is there a "common repo" for the boilerplate newsfragments/README.md file? Turns out fragments from the misc category don't get their contents displayed, they just get the numbers listed in a "Miscellaneous internal changes" section.

Christoph Burgdorf

@cburgdorf

@veox if those are separate categories (e.g performance and feature) then you can create two individual files. If those are of the same category then I'm unaware of differentiating them apart from line breaks.

@veox eventually it will make it into the ethereum-python-project-template ethereum/ethereum-python-project-template#28 but we wanted to wait for the next Trinity release to see if there are any further tweaks to the release script needed.

Noel Maersk

@veox

Thanks, looks like just the thing.