Use ethash_blockhash_t in the codebase #15
Conversation
|
A lot of these refactorings break ethash.go You might consider getting |
|
yes, there is a reason this is not merged yet. Lack of proper continuous integration testing for this repo does not help. |
- Adding tests for the functionality offered by ethash_io - ethash_io_write() now accepts ethash_params and seedhash instead of a block number. This is for better modularity but first and foremost for testability via unit tests
- Using a typedef struct instead of passing an array of hard coded length of 32 bytes everywhere. It's much better C practise and also gives us typechecking. - Also corrected style in places I touched. I think a style PR should follow after that.
d1e4e48 to
fbd36b0
Compare
|
Rebased on top of master. Was a pain and will need to fix stuff. |
436c6ac to
eebb752
Compare
|
It looks like you need to refactor pyethash. Incidentally, the javascript implementation is broken. I started working on fixing it but my progress got reverted. It's probably a good idea to make an npm module that wraps the C code to enable mining on the testnet with node-ethereum. |
|
yeah just noticed that the python stuff fails now. One fix per commit :) We will see what to do about the JS implementation. The suggestion sounds nice but I am no javascript expert so I would have to investigate further. The most important thing is for C++ and Go to operate properly though. The rest is just a "good to have" feature as far as priorities are concerned. |
|
In that case you might consider deleting (1) On non-SDD drives it takes longer to load the DAG from disk than it does to build it in memory from scratch Better to have javascript support, since go and cpp-ethereum have too many requirements for most developers to deal with. |
|
The plan is for DAG file writting to move to the C library. This is the whole reason this thing started. Go will switch to using it too once the API settles. Feel free to refactor the Javascript stuff and to implement your suggestion and open a PR. I am sure it would be really appreciated. |
- Also added some special cases in python/test.sh for ArchLinux and made sure that the nose package of the virtualenv is used even if the system already has one
|
Incidentally, you are going to have to update pypi with the new module information for pyethash after this refactoring. (or not, since all you've really done here is bikeshedding) |
|
yes I did not make any API changes here, just made python/core compile for the changes this PR is introducing. |
|
The Python 3 stuff requires a push to pypi, so I'll raise an issue. |
- More robust virtualenv version detection - Versioning adjustment as dictated in https://www.python.org/dev/peps/pep-0440/ after suggestions by @heikoheiko - Pep8 style changes in setup.py
|
Note: Please do not merge until go tests also pass. Following the methodology we use in cpp-ethereum I will put the PR in the "please review" state. |
- @obscuren please have a look. This PR makes some refactoring changes in the C code which replace all uint8_t[32] arrays by a typedeffed struct. I made some changes in the ethash.go code which are meant to adjust for that.
|
gotests now pass with the latest commit. Have to manually run them since they are not integrated into travis yet. |
|
Go changes are fine |
Use ethash_blockhash_t in the codebase
Depends on #11.
length of 32 bytes everywhere. It's much better C practise and also
gives us typechecking.
follow after that.