Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

"python ./runtest.py" is broken #7

Closed
freizeit opened this issue Feb 5, 2014 · 29 comments
Closed

"python ./runtest.py" is broken #7

freizeit opened this issue Feb 5, 2014 · 29 comments

Comments

@freizeit
Copy link
Contributor

freizeit commented Feb 5, 2014

Here's what I see:

$ python ./runtest.py 
Traceback (most recent call last):
  File "./runtest.py", line 34, in <module>
    t0.update(k,x[k])
TypeError: string indices must be integers

This is on ubuntu 12.10

Linux dwtf 3.11.0-15-generic #25-Ubuntu SMP Thu Jan 30 17:22:01 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

$ python --version
Python 2.7.5+
@uwecerron
Copy link
Contributor

it appears it's a json parsing problem. treat it as a dictionary?

@freizeit
Copy link
Contributor Author

freizeit commented Feb 5, 2014

yeah .. looking into it now

@freizeit
Copy link
Contributor Author

freizeit commented Feb 5, 2014

off-topic question: is there an an irc channel where ethereum devs hang out?

@uwecerron
Copy link
Contributor

not that i know of, try #ethereum or #ethereum-dev

@guaka
Copy link
Contributor

guaka commented Feb 21, 2014

Here's what I get on OSX:

pyethereum $ python ./runtest.py
Traceback (most recent call last):
  File "./runtest.py", line 7, in <module>
    rlpdata = json.loads(open(os.path.join(testdir,'rlptest.txt')).read())
IOError: [Errno 2] No such file or directory: '../tests/rlptest.txt'

I can't quickly find out where this rlptest.txt is supposed to come from.

(and yes both #ethereum and #ethereum-dev is active)

@heikoheiko
Copy link
Member

You need to clone the "tests" which can be found here: https://github.com/ethereum/tests

@sunny-g
Copy link
Contributor

sunny-g commented Feb 21, 2014

OSX Mavericks is throwing this error:

$ pwd
$HOME/pyethereum/tests       // I cloned ethereum/tests into tests
$ python ../runtest.py
Mismatch with adds only ("{u'dogglesworth': u'cat', u'dog': u'puppy', u'doe': u'reindeer'}" -> "cf7d318935b52db6e23d8c1f5e6b7e62f3606d4ed13783f4fdbd6e67a2085d04")
Mismatch with adds only ("{u'do': u'verb', u'horse': u'stallion', u'doge': u'coin', u'dog': u'puppy'}" -> "0d9ce42de1d9e290181283ff8168f9dc40b93de6d8a06f55670290881cc95f2d")

@freizeit
Copy link
Contributor Author

That's expected -- the runtest.py script was fixed but the test data in the "tests" repository wasn't updated.

Please note also that a large part of runtest.py was ported to "proper" python tests (see the "tests" subdirectory). These use the same test data and will not pass either yet.

In a next step I'd like to drop the dependency on the external "tests" repository and have the test data reside local to "pyethereum".

@konradkonrad
Copy link
Contributor

Could someone help me out here with all the tests involved?

@freizeit in #11 you introduce py.test and you reference this (#7) as fixed. However it is still open and runtest.py is still in the repo. Furthermore in #12 you reference #7 as fixed.

@chenhouwu In commit 4703540 you introduced 'behave' as new test framework, but runtest.py is still in the repository.

If I understand correctly, py.test and runtest.py still rely on cloning the tests repo (and fail if not cloned)?

So...,

  • is there any clear roadmap towards a single test framework?
  • when creating PR's, which tests do my commits need to pass?

Thanks!

@laishulu
Copy link
Contributor

Currently only test for rlp is implemented in behave,
so run behave will only test rlp.

trie and others are still same as before, but it surely will be migrated to behave later.
so currently runtest.py shoulde be modified now to get rid of the codes for rlp testing

@laishulu
Copy link
Contributor

runtest.py is obsolete and removed

@al-maisan
Copy link

Hello there! Sorry I could not answer all the questions in time. My understanding is that the rlp module is broken and does not comply to https://github.com/ethereum/wiki/wiki/%5BEnglish%5D-RLP

I am in the process of preparing a branch that will address some of this.

Could you all please also join https://groups.google.com/forum/#!forum/ethereum-dev so we can discuss and agree on development issues? Thank you very much indeed!

@al-maisan
Copy link

@konradkonrad yes, the py.test tests are still cloning the "tests" repository. In the "rlp fixes" branch (currently in progress) I test against the "develop" branch.

However, I have doubts re. the following values in https://github.com/ethereum/tests/blob/develop/rlptest.json

  • "zero"
  • "bigint"

and will prepare an email to the ethereum-dev to discuss this.

Regarding test frameworks: I have used py.test with good success in the past and know the author well -- hence my inclination to use it for pyethereum as well.

@laishulu
Copy link
Contributor

I've joined the google groups :-).

rlp encoding would only encode string (ie. byte array) and list.
So an integer first should be converted to a string.
Thus the encoding result depends on how it's converted.

If '{0}'.format(zero) then the result will be \x30,
but if chr(zero), the result would be \x00

@heikoheiko
Copy link
Member

In the Ethereum(++) client \x00 is encoded as empty string "" when sending rlp encoded data over the network. Therefore "zero" and "emptystring" have the same value "80".

@heikoheiko
Copy link
Member

Here's how utils.py should look like when encoding "zero" as the empty string.
https://gist.github.com/heikoheiko/9727917

@al-maisan
Copy link

@heikoheiko I understand now -- thanks for the clarification !

@al-maisan
Copy link

Another question: encoding as follows is clear:

int -> byte buffer -> RLP

However, the other way around

RLP -> byte buffer

How would I know whether the byte buffer should further be transformed into an integer or left as is?

@heikoheiko
Copy link
Member

Further transformation depends on the application. You basically have to know what data and types you expect to get out of the deserialization.

For example if you get ['Bob', '7', ''] from rlp.decode, and know that you expect (str:name, int:account, int:balance) then you would have to treat the last two elements as big-endian encoded integers and finally get ['Bob', 55, 0].

@laishulu
Copy link
Contributor

@al-maisan , sorry, just mix the form of '%d' % (0) and '{0}'.format(0) in the previous post.
Already fixed now

@heikoheiko, the utils.py fike is created by me,

de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800  1) def int_to_big_endian(integer):
de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800  2)     '''convert a integer to big endian binary string
de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800  3)     '''
de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800  4)     s = '%x' % integer
f2b796b9 utils.py (Chen Houwu 2014-03-17 16:04:36 +0800  5)     if len(s) & 1:
f2b796b9 utils.py (Chen Houwu 2014-03-17 16:04:36 +0800  6)         s = '0' + s
f2b796b9 utils.py (Chen Houwu 2014-03-17 16:04:36 +0800  7)     return s.decode('hex')
de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800  8)
de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800  9) def big_endian_to_int(string):
de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800 10)     '''convert a big endian binary string to integer
de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800 11)     '''
de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800 12)     s = string.encode('hex')
de5d6c98 utils.py (Chen Houwu 2014-03-18 12:57:42 +0800 13)     return int(s, 16)

I don't give the special case when the value is 0.
So the following codes are not there

if integer == 0:
  return '' 

@laishulu
Copy link
Contributor

@al-maisan for RLP encoding, it only accept string(i.e raw byte) or a list of string. RLP itself is designed to work in low level without the know what the strings means. So how the high level data is serialized to string and deserialized from string is totally up to you.

Hope it's clarified now.

@heikoheiko
Copy link
Member

@chenhouwu in order to be compatible with the Ethereum(++) client (which is the most current spec) we need to treat the special case of zero/empty_string. i.e. big_endian_to_int("") will fail. I fixed this here: https://github.com/brainbot-com/pyethereum/blob/d2e166a6b8196e0cc24bf6f2fe2dcaab3a578506/pyethereum/utils.py

@laishulu
Copy link
Contributor

@heikoheiko yes, blank string will be encoded to \0x80. but for zero, first we should convert it into byte array. so here you mean zero is converted to ''. but 1 or 2 or some other integers are not.
That's inconsistent obviously.

@laishulu
Copy link
Contributor

in fact, in the RLP encoding spec, it does not care about how you convert integer in high level codes.
For integer, it only takes care of the result of "payload length plus some prefix(like 0x80, 0xc0...)"

So in the low level RLP encoding, it will never meet the case of convert 0 to big endian array.

@laishulu
Copy link
Contributor

@al-maisan

If '{0}'.format(zero) then the result will be \x30(not \x80),
but if chr(zero), the result would be \x00.

So, why the \x80 comes up, I don't know.
I'll discuss it with @vbuterin further

@al-maisan
Copy link

@chenhouwu can you please take that discussion to https://groups.google.com/forum/#!forum/ethereum-dev so other people see it as well and we document the outcome and the discussion? Thanks!

P.S.: @vbuterin has subscribed to that list as well

@vbuterin
Copy link
Contributor

The way things should be:

rlp.encode(0) = rlp.encode("") = '\x80'
rlp.encode(chr(0)) = rlp.encode("\x00") = '\x00'
rlp.encode([]) = '\xc0'

There is no need to special case zero; it's just a natural extension of the rules for nonzero length strings. For integer conversion, 0 -> '' is just the base case.

@laishulu
Copy link
Contributor

@vbuterin
Still not clear about the conversion, why rlp.encode(0) = rlp.encode("")
i.e. in what rule integer 0 is converted to ""?
also, what's the result of rlp.encode(10)?

@laishulu laishulu reopened this Mar 26, 2014
@laishulu
Copy link
Contributor

laishulu commented Apr 1, 2014

From @vbuterin

rlp.encode(x) = chr(x) holds for 1 <= x <= 255

and rlp.encode(0) = rlp.encode("") is a pre-determined case.

@laishulu laishulu closed this as completed Apr 1, 2014
joeykrug added a commit that referenced this issue Dec 8, 2017
enabled snapshot usage again
changwu-tw pushed a commit to changwu-tw/pyethereum that referenced this issue Jan 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants