feat(cactus-plugin-ledger-connector-ethereum): add new connector plugin #2535

outSH · 2023-07-04T15:17:44Z

Add new plugin for connecting with Ethereum ledgers. New connector is based on already existing quorum connector. The main reason for introducing yet another plugin is a need for web3js upgrade (to 4.X) which is imposible in current quorum / besu connectors due to dependency to web3js-quorum which requires web3js 1.X.

We have plans to make web3js library pluggable and reduce code duplication among other connectors in the future, but it will be delivered later on in a separate PR.

Changes:

Add new plugin based on quorum connector.
Removed private transaction and other quorum related functionalities.
Update web3js to 1.10 - will be updated to 4.X in a separate commit.
Add missing web3-eth-contract dependencies to besu and xdai connectors.
Add new connector to cactus-verifier-client
Add integration tests in cactus-test-plugin-ledger-connector-ethereum (based on similar ones for quorum connector.)
Add new connector to CI.
Add web3* 1.5.2 dependencies to root package.json because they are already required in a root level (by typings/web3js-quorum). Ideally this could be put into another package (quorum connector?) and have the dependencies there, but for now I think it's important to be explicit about it since it's easy to mess up if wrong web3js library is hoisted up from any monorepo package.
Sort main package.json
Remove tap test scripts from the root package.json - they don't use .taprc and cause bunch of errors when they try to execute jest tests. This is confusing for the users because of all false negative errors printed.
Reorganize jest config and taprc to keep tests from quorum and ethereum conenctors grouped.

Closes: #2534

...tus-plugin-ledger-connector-ethereum/src/main/typescript/plugin-ledger-connector-ethereum.ts

petermetz

@outSH Looks legit, I just have some notes regarding 2 other PRs in parallel:

Is upgrading web3 project-wide to a newer version than what you are using here so please be on guard about that getting merged first (if it does - I don't know for sure it will)
There's another PR in parallel also, the one with the yarn v3 upgrade and so far it is looking like we'll have to move away from hoisting completely because of the way it makes a lot of hard to debug bugs very easy to create the way you are also explaining it in your PR description.

outSH · 2023-07-13T11:06:21Z

@petermetz Thanks!

ad1. You mean https://github.com/hyperledger/cacti/pull/2550? I'm updating to more recent one here (1.10 > 1.6.1). Either way, if I filled all missing web3 packages we should be good to go but I'll retest it when the PR you mentioned is merged :)
ad2. Sounds great!

izuru0

LGTM

outSH · 2023-07-14T14:14:13Z

@petermetz @izuru0 Just a quick note - I plan on changing the connector API during the web3js update (that I'm currently working on), so please don't merge it until release 2.0 (if that would make my future PR problematic). Ideally, I'd like to mark this connector as "API unstable" in README and make API breaking changes until futher notice but not sure if you'd agree on that :)

petermetz · 2023-07-15T17:10:56Z

ad1. You mean #2550? I'm updating to more recent one here (1.10 > 1.6.1).

@outSH Oh, my bad, I was looking at some other part of the diff and saw 1.5.2 [1] but I see now that you just did that because that was the version in those packages. Alright, we'll make it work one way or another, whoever merges second will need a little update on the versions to make sure it's all good. No action necessary, just raising awareness for now.

[1] https://github.com/hyperledger/cacti/pull/2535/files#diff-74173a496c0c85400952267e63584f4de82801399bb7ec3acdca99ede1c4d0e9

petermetz · 2023-07-15T17:20:23Z

@petermetz @izuru0 Just a quick note - I plan on changing the connector API during the web3js update (that I'm currently working on), so please don't merge it until release 2.0 (if that would make my future PR problematic). Ideally, I'd like to mark this connector as "API unstable" in README and make API breaking changes until futher notice but not sure if you'd agree on that :)

The easiest way for me to handle that is to put this in draft state and then we can keep it open as long as you like.
The downside for that is that you'll have to keep rebasing and fixing the merge conflicts that come up while main is getting more changes in. If you are okay with that, then I'd just keep it open until it's stable enough or just wait until 2.0.

I'll put a 'change request' on this just to stop us from accidentally merging it because it's been already approved so I was about to merge it before you made that comment!

If you want to avoid keeping this one open for long and then spend time rebasing then there are a couple more options that I can think of:

You version with v1 whichever endpoint & method name(s) you need to and then instead of making a breaking change on those with your later PR, instead you just create the v2 versions from scratch. There's code duplication this way but with re-using the common parts it can be kept low enough that it's worth it IMO and then this way there are no breaking changes.
You set the package.json's publish config to private so that lerna ignores it when the batch publishing of all the packages is happening. Doing this might cause issues with the release scripts (it has in the past) so it's my least preferred option but still possible.

petermetz

Not an actual change request. I'm just blocking the merge for now. See the discussion above for details.

- 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: hyperledger-cacti#2580 Depends on hyperledger-cacti#2535 Depends on hyperledger-cacti#2578 Signed-off-by: Michal Bajer <[email protected]>

jagpreetsinghsasan

LGTM, not approving for the same reasons as @petermetz mentioned

- 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: hyperledger-cacti#2580 Depends on hyperledger-cacti#2535 Depends on hyperledger-cacti#2578 Signed-off-by: Michal Bajer <[email protected]>

outSH · 2023-08-23T15:04:59Z

Just a quick note - I plan on changing the connector API during the web3js update

@petermetz
With https://github.com/hyperledger/cacti/pull/2631 I'm done with API changes, so all PRs in chain can be merged. Let me know if this can be done by the release of V2, I will start rebasing and clearing out the PRs in order then.

petermetz · 2023-08-25T06:01:50Z

@petermetz
With https://github.com/hyperledger/cacti/pull/2631 I'm done with API changes, so all PRs in chain can be merged.

@outSH
Nice! Thank you very much for those contributions!

Let me know if this can be done by the release of V2, I will start rebasing and clearing out the PRs in order then.

Yes! They all can and should in my opinion because we opened up the main branch for breaking changes as of last week => meaning that there will be no more 1.x releases (from the main branch) issued anymore so we can start making all the breaking changes that we've been holding off until now.
So, now is the time to "be free" so please double check if there's any other breaking changes you wanted to make in the past because between now and the 2.0.0-GA we can actually make it happen, no problem. The next time we'll have this chance is the last few weeks for v3.0.0.

petermetz

Approving now since we are ready to go! Thanks again @outSH !

petermetz · 2023-08-31T16:49:11Z

FYI: I dismissed the false-positive CodeQL alerts because we do have our own runtime checks in place to make sure that there are no invalid method names provided when calling contract methods.

Add new plugin for connecting with Ethereum ledgers. New connector is based on already existing quorum connector. The main reason for introducing yet another plugin is a need for web3js upgrade (to 4.X) which is imposible in current quorum / besu connectors due to dependency to `web3js-quorum` which requires web3js 1.X. We have plans to make web3js library pluggable and reduce code duplication among other connectors in the future, but it will be delivered later on in a separate PR. Changes: - Add new plugin based on quorum connector. - Removed private transaction and other quorum related functionalities. - Update web3js to 1.10 - will be updated to 4.X in a separate commit. - Add missing `web3-eth-contract` dependencies to besu and xdai connectors. - Add new connector to cactus-verifier-client - Add integration tests in `cactus-test-plugin-ledger-connector-ethereum` (based on similar ones for quorum connector.) - Add new connector to CI. - Add `web3*` 4.1.1 dependencies to root `package.json` because they are already required in a root level (by `typings/web3js-quorum`). Ideally this could be put into another package (quorum connector?) and have the dependencies there, but for now I think it's important to be explicit about it since it's easy to mess up if wrong web3js library is hoisted up from any monorepo package. - Sort main `package.json` - Remove tap test scripts from the root `package.json` - they don't use `.taprc` and cause bunch of errors when they try to execute jest tests. This is confusing for the users because of all false negative errors printed. - Reorganize jest config and taprc to keep tests from quorum and ethereum conenctors grouped. Peter's tweaks: 1. Switched the root's web3 dependencies from 1.5.2 to 4.1.1 because in the meantime we've done a few upgrades (plus the old versions come with CVEs) 2. There is no more hoisting, it's been completely disabled, so we should not have any more issues regarding that. It is still true however, that the root folder has our handwritten type definitions which do import web3 packages and therefore the root should declare web3 as a dependency. 3. Once https://github.com/hyperledger/cacti/issues/2648 has been resolved we can remove both the root dependency declarations of web3 and the typings that necessitate them to begin with. Closes: hyperledger-cacti#2534 Co-authored-by: Peter Somogyvari <[email protected]> Signed-off-by: Michal Bajer <[email protected]> Signed-off-by: Peter Somogyvari <[email protected]>

- 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. Closes: hyperledger-cacti#2580 Depends on hyperledger-cacti#2535 Depends on hyperledger-cacti#2578 Signed-off-by: Michal Bajer <[email protected]>

- 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. Closes: hyperledger-cacti#2580 Depends on hyperledger-cacti#2535 Depends on hyperledger-cacti#2578 Co-authored-by: Peter Somogyvari <[email protected]> Signed-off-by: Michal Bajer <[email protected]> Signed-off-by: Peter Somogyvari <[email protected]>

- 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. Closes: #2580 Depends on #2535 Depends on #2578 Co-authored-by: Peter Somogyvari <[email protected]> Signed-off-by: Michal Bajer <[email protected]> Signed-off-by: Peter Somogyvari <[email protected]>

- 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. Closes: hyperledger-cacti#2580 Depends on hyperledger-cacti#2535 Depends on hyperledger-cacti#2578 Co-authored-by: Peter Somogyvari <[email protected]> Signed-off-by: Michal Bajer <[email protected]> Signed-off-by: Peter Somogyvari <[email protected]>

outSH requested review from petermetz, takeutak, izuru0, jagpreetsinghsasan, VRamakrishna and sandeepnRES as code owners July 4, 2023 15:17

outSH force-pushed the ethereum_connector branch from e7939f7 to a29b8b9 Compare July 4, 2023 15:42

github-advanced-security bot found potential problems Jul 4, 2023

View reviewed changes

petermetz approved these changes Jul 12, 2023

View reviewed changes

izuru0 approved these changes Jul 14, 2023

View reviewed changes

petermetz self-requested a review July 15, 2023 17:20

petermetz requested changes Jul 15, 2023

View reviewed changes

outSH force-pushed the ethereum_connector branch from a29b8b9 to 1a475cb Compare July 25, 2023 11:30

outSH mentioned this pull request Jul 25, 2023

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

Merged

jagpreetsinghsasan reviewed Aug 3, 2023

View reviewed changes

outSH requested a review from petermetz August 23, 2023 15:05

petermetz approved these changes Aug 25, 2023

View reviewed changes

outSH force-pushed the ethereum_connector branch from 1a475cb to cd3a9d8 Compare August 29, 2023 13:41

petermetz force-pushed the ethereum_connector branch from cd3a9d8 to bc7d96f Compare August 31, 2023 16:58

petermetz enabled auto-merge (rebase) August 31, 2023 16:58

petermetz force-pushed the ethereum_connector branch from bc7d96f to c011c36 Compare August 31, 2023 21:55

petermetz merged commit f8a2131 into hyperledger-cacti:main Aug 31, 2023
121 of 132 checks passed

petermetz mentioned this pull request Jun 14, 2024

chore(release): publish v2.0.0-rc.1 #3324

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cactus-plugin-ledger-connector-ethereum): add new connector plugin #2535

feat(cactus-plugin-ledger-connector-ethereum): add new connector plugin #2535

outSH commented Jul 4, 2023

petermetz left a comment

outSH commented Jul 13, 2023

izuru0 left a comment

outSH commented Jul 14, 2023

petermetz commented Jul 15, 2023

petermetz commented Jul 15, 2023

petermetz left a comment

jagpreetsinghsasan left a comment

outSH commented Aug 23, 2023

petermetz commented Aug 25, 2023

petermetz left a comment

petermetz commented Aug 31, 2023

feat(cactus-plugin-ledger-connector-ethereum): add new connector plugin #2535

feat(cactus-plugin-ledger-connector-ethereum): add new connector plugin #2535

Conversation

outSH commented Jul 4, 2023

petermetz left a comment

Choose a reason for hiding this comment

outSH commented Jul 13, 2023

izuru0 left a comment

Choose a reason for hiding this comment

outSH commented Jul 14, 2023

petermetz commented Jul 15, 2023

petermetz commented Jul 15, 2023

petermetz left a comment

Choose a reason for hiding this comment

jagpreetsinghsasan left a comment

Choose a reason for hiding this comment

outSH commented Aug 23, 2023

petermetz commented Aug 25, 2023

petermetz left a comment

Choose a reason for hiding this comment

petermetz commented Aug 31, 2023