search

status-im / nim-eth-p2p

Nim Ethereum P2P protocol implementation
Apache License 2.0
11 stars 4 forks source link

Add snappy compression to RLPx (EIP-706) #28

Closed data-man closed 5 years ago

data-man commented 6 years ago

Not working yet. I want to make sure that I'm on the right way. :-)

cheatfate commented 6 years ago

It looks like your wrapper is unsafe.

zah commented 6 years ago

@cheatfate, can you clarify what you noticed about the wrapper?

@data-man, the code seems to insert if blocks in all the right places, but it's not clear to me what the issues were. I would recommend that you change the input types of the libsnappyc package to use openarray[byte] though.

cheatfate commented 6 years ago

@zah, please check Appveyor tests.

data-man commented 6 years ago

@zah

but it's not clear to me what the issues were.

Just not working as expected. :-(

I would recommend that you change the input types of the libsnappyc package to use openarray[byte] though.

I plan to port Snappy to pure Nim, but I'm not sure where: in the status-im or in NimCompression?

zah commented 6 years ago

Well, do we assume that the C port of snappy has such basic issues because it's less often used? Seems a bit dubious to me, but nevertheless, we can also try the classic snappy lib, which actually also has a C interface:

https://github.com/google/snappy/blob/master/snappy-c.h

data-man commented 6 years ago

After adding the CIs tests, I discovered that the snappy-c doesn't work on x32.