Snipa22 / nodejs-pool

Other
481 stars 421 forks source link

Monero v15 compatibility: "Failed to parse block" in convert_blob() after hardfork. #496

Closed osensei closed 1 year ago

osensei commented 1 year ago

This actually belongs to https://github.com/Snipa22/node-cryptonote-util/ , which is the module this pool software is using, but since issue reporting seems to be disabled on that repo, I'm opening it here.

Although I'm running my own pool code based originally on the old zone117x/node-cryptonote-pool software, I've found this problem with both https://github.com/Snipa22/node-cryptonote-util and https://github.com/MoneroOcean/node-cryptoforknote-util, so I'm pretty sure this pool software will also face the same problem.

I've been doing some tests on a private testnet to ensure everything will work after the v15 fork.

As soon as the hardfork takes place, convert_blob() will break with the error message "Failed to parse block".

Although there's no PoW change on v15, it is my understanding that the block template is changing to include the new feature "view tags" (and maybe something else) and this might be breaking things.

I've already reported it on MoneroOcean's repo and some work is being done on it: https://github.com/MoneroOcean/node-cryptoforknote-util/issues/19 Sadly, I can't test the changes due to unmet dependencies in my environment.

MoneroOcean's fork includes a lot of code related to other currencies that has been added over the past few years. I would be grateful if someone could backport the needed changes to https://github.com/Snipa22/node-cryptonote-util/tree/xmr-Nan-2.0 to keep it simple without new dependencies, and hopefully be able to build it in my current environment.

Let me know if there's anything I can help you with.

j-berman commented 1 year ago

Can you give this a try? https://github.com/j-berman/node-cryptonote-util/commits/hf-15

Should be everything needed but haven't tested it yet. If you've got instructions on how to set up a private testnet with nodejs-pool running this code that would be appreciated.

osensei commented 1 year ago

Hey, thanks for this.

I've tried building it but I'm getting these 2 errors, the second one being repeated a few of times:

../src/cryptonote_core/cryptonote_format_utils.cpp:116:19: error: ‘set_tx_out’ is not a member of ‘cryptonote’
../src/serialization/serialization.h:36:14: error: ‘class crypto::view_tag’ has no member named ‘do_serialize’

I'm attaching the entire output. output1.txt

I also tried running the following commands in clean instances with ubuntu bionic and ubuntu focal to discard it had anything to do with my outdated environment, and although the warnings may differ, the errors are still the same:

sudo apt install nodejs npm libboost-all-dev
mkdir test
cd test
npm install https://github.com/j-berman/node-cryptonote-util#hf-15

Regarding the private testnet, I first synced up the testnet blockchain.

Then ran: ./monero-blockchain-import --testnet --pop-blocks <number_of_blocks> to pop the amount of blocks needed to go back to 1 or 2 blocks before testnet v15 hardfork block height (1982800)

Then ran monerod like this: ./monerod --testnet --no-igd --rpc-bind-port 28083 --rpc-bind-ip 0.0.0.0 --confirm-external-bind --hide-my-port --offline

And then started mining on the pool to see how it would behave before and after the fork.

I'm sorry I cannot give you precise instruction on how to set up nodejs-pool other than what's in the README. As I mentioned before I'm running my own fork based on another "first-generation" pool software: zone117x/node-cryptonote-pool, but that one is pretty outdated.

That being said, I'm more than happy to keep testing and giving feedback.

If you can build it with npm install https://github.com/j-berman/node-cryptonote-util#hf-15 I believe I should be able to try it out.

j-berman commented 1 year ago

Sorry about that -- it's building now. Will work on setting up a private testnet on my end too.

osensei commented 1 year ago

Thank you very much!

I can verify it works now, before and after the fork!

Snipa22 commented 1 year ago

Thanks for the PR, and testing osensei, I've verified this patch works and has been applied back into the main repo.