feat(cactus-plugin-ledger-connector-ethereum): update web3js to 4.X

[outSH]

• Update web3js packages from 1.10 to 4.0.3 in cactus-plugin-ledger-connector-ethereum and
  cactus-test-plugin-ledger-connector-ethereum. This allows interacting
  with most recent geth nodes.
• Refactor all ethereum tests. Most of the test cases were duplicated multiple times
  (between different quorum ledger versions test and deployment methods). I've removed all this
  duplication while maintaining similar level of test coverage.
  New tests use Geth test ledger instead of Quorum one.
• Add web3js type conversions methods to minimize impact of poor dynamic typing
  in this early release of 4.X.
• Update API. In 4.X all numeric responses has been converted to BigNum.
  To keep up with this some fields has been changed to string instead of number when necessary.
  Add some missing fields as well.
• Add estimateMaxFeePerGas method for estimating current transaction cost.
• Fix invalid runTransact response type.
• Add test script for checking integration with Alchemy that must be executed manually
  (it's excluded from CI at the moment) - geth-alchemy-integration-manual-check.test.
  Instructions on how to run it has been added to package README.

Future improvements:

• Support London fork gas fees (i.e. EIP-1559)
• Refactor API to allow future extensions.
• Fix several TODO items in this connector.

Closes https://github.com/hyperledger/cacti/issues/2580
Closes https://github.com/hyperledger/cacti/issues/2656
Closes https://github.com/hyperledger/cacti/issues/2594

Depends on https://github.com/hyperledger/cacti/pull/2535
Depends on https://github.com/hyperledger/cacti/pull/2578

Signed-off-by: Michal Bajer [email protected]

[outSH]

Please review only the latest commit (others are dependencies).

Note to myself: Update references to geth image once it's available, remove outsh from cspell ignore file.

[github-actions]

This PR/issue depends on:

• hyperledger/cacti#2535
• hyperledger/cacti#2578
  By Dependent Issues (🤖). Happy coding!

[petermetz]

@outSH The cactus-plugin-ledger-connector-ethereum job is failing, could you please check on it?

Attaching the logs in case they expire in the meantime on the backend:
2023-09-18T12-42-58-cacti-ci-test-connector-ethereum-crash.log

[outSH]

@outSH The cactus-plugin-ledger-connector-ethereum job is failing, could you please check on it?

Attaching the logs in case they expire in the meantime on the backend: 2023-09-18T12-42-58-cacti-ci-test-connector-ethereum-crash.log

Yes, I will request a review once this PR is ready for merge, didn't address previous comments yet as well

[outSH]

@outSH The cactus-plugin-ledger-connector-ethereum job is failing, could you please check on it?

Attaching the logs in case they expire in the meantime on the backend: 2023-09-18T12-42-58-cacti-ci-test-connector-ethereum-crash.log

@petermetz Both test suites for this connector should be passing from now on. Sorry for breaking the CI with previous PR, I wasn't checking the test results because I kinda assumed you're doing it anyway :) I should have make sure they pass before even requesting a review.

I've also fixed https://github.com/hyperledger/cacti/issues/2594 while working on cactus-test-plugin-ledger-connector-ethereum test. There was a race condition - web3js socket was being disconnected before watchBlock eth_unsubscribe request was sent. Ledger didn't know it was OK and was closing the connection again, causing unhandled error.

Now, the shutdown method will first unsubscribe all remaining watchBlocks (in case the client was not disconnected yet), wait for all pending requests to pass and then disconnect the provider. To see request queue I had to access protected field of WS Provider - I will create an issue on web3js github to improve this (either handle it on web3js side or export some public field like queueLength). I hope this is acceptable for now and we can improve on that on future PRs.

One more change I did was adding wsApi.disconnectSockets(true) to ApiServer shutdown. Reasoning: Socketio connections should be closed before shuting down the plugins (we don't want to handle new async requests while working on clearing up resources). This could possibly be extended to HTTP too, that is: stop connections, stop the servers, don't accept new connection and only then do plugin shutdowns, but I didn't want to introduce such change in this PR (closing of connections is more urgent).

BTW I wanted to include closing of socketio connections in the connector, but I can't do that since in theory single connection can be used to access multiple plugins (they are api-server responsibility, not connectors). We could use socketio rooms to partition the sockets and move overnship of the active connection to the connector, but I'm not sure if it's beneficial given our time contraint.

[petermetz]

I kinda assumed you're doing it anyway :)

@outSH Hehe, I usually do, rigorously, but with the upcoming v2.0.0 release I've been able to spend a little less time on these things as well TBH. I'm glad you noticed though, thank you again! :-)

I've also fixed https://github.com/hyperledger/cacti/issues/2594 while working on cactus-test-plugin-ledger-connector-ethereum test. There was a race condition - web3js socket was being disconnected before watchBlock eth_unsubscribe request was sent. Ledger didn't know it was OK and was closing the connection again, causing unhandled error.

Now, the shutdown method will first unsubscribe all remaining watchBlocks (in case the client was not disconnected yet), wait for all pending requests to pass and then disconnect the provider. To see request queue I had to access protected field of WS Provider - I will create an issue on web3js github to improve this (either handle it on web3js side or export some public field like queueLength). I hope this is acceptable for now and we can improve on that on future PRs.

Sounds good to me, thank you so much for going the extra mile and investigating the race condition! It makes things easier down the line! It's a reasonable trade-off IMO as well and hopefully web3js will provide some method for us to use that makes this trivial to implement.

One more change I did was adding wsApi.disconnectSockets(true) to ApiServer shutdown. Reasoning: Socketio connections should be closed before shuting down the plugins (we don't want to handle new async requests while working on clearing up resources). This could possibly be extended to HTTP too, that is: stop connections, stop the servers, don't accept new connection and only then do plugin shutdowns, but I didn't want to introduce such change in this PR (closing of connections is more urgent).

LGTM on this too, we can refactor later if it's no longer needed! Thank you!

BTW I wanted to include closing of socketio connections in the connector, but I can't do that since in theory single connection can be used to access multiple plugins (they are api-server responsibility, not connectors). We could use socketio rooms to partition the sockets and move overnship of the active connection to the connector, but I'm not sure if it's beneficial given our time contraint.

Let's talk more about this in an issue that I'll try to open (and not forget)!

[petermetz]

LGTM

[izuru0]

LGTM