Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Web3J #3752

Merged
merged 21 commits into from
Aug 4, 2022
Merged

Upgrade Web3J #3752

merged 21 commits into from
Aug 4, 2022

Conversation

diega
Copy link
Contributor

@diega diega commented Apr 22, 2022

ON HOLD: waiting for org.web3j:quorum release (ref: web3j-quorum#66)

Update Web3J dependency

This PR also reverts #3789

Signed-off-by: Diego López León dieguitoll@gmail.com

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

@diega diega marked this pull request as draft April 22, 2022 13:30
@diega diega mentioned this pull request Apr 22, 2022
2 tasks
@diega diega force-pushed the bump_web3j branch 2 times, most recently from 6afbac1 to 6c405ea Compare May 11, 2022 12:32
@sonarcloud
Copy link

sonarcloud bot commented May 11, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@diega
Copy link
Contributor Author

diega commented Jul 12, 2022

@antonydenyer to move this PR to 4.9.3 we are missing org.web3j:quorum which is on 4.9.0. Also the web3j-gradle-plugin would be great if can be moved to 4.9.3 (it's on 4.9.2). Should I fill an issue in the respective projects?
This would be an attempt to check if we can make the acceptanceTests to pass using those versions

@antonydenyer
Copy link
Member

@antonydenyer to move this PR to 4.9.3 we are missing org.web3j:quorum which is on 4.9.0. Also the web3j-gradle-plugin would be great if can be moved to 4.9.3 (it's on 4.9.2). Should I fill an issue in the respective projects? This would be an attempt to check if we can make the acceptanceTests to pass using those versions

I think raising an issue in the respective projects would make sense.

I'm happy to have a look at the acceptanceTests first, before looking at the version inconsistencies.

I think we should be able to use everything on to 4.9.3 and have org.web3j:quorum and web3j-gradle-plugin forced to use the 4.9.3 dependencies bit life would be a lot easier if upstream libraries were updated.

@antonydenyer
Copy link
Member

Blocked by hyperledger-web3j/web3j#1747

@diega
Copy link
Contributor Author

diega commented Jul 19, 2022

While we wait for a release of the web3j components to fix the acceptanceTests, I tried to force the dependencies as suggested.
For org.web3j:quorum it seems to possible by using constraints, but web3j-gradle-plugin forces the inclusion of the org.web3:core:$projectVersion dependency programmatically (see o.w.g.p.Web3jPlugin:49) so we end up using both versions during the build/test cycle. Fortunately this is not so terrible as both cycles have different classpaths, at build v4.9.0 will be used just for the plugin, and at runtime +v4.9.0 will be used for any other project usage.

@diega diega changed the title Web3J 4.9.0 Upgrade Web3J Jul 19, 2022
Signed-off-by: Diego López León <dieguitoll@gmail.com>
This reverts commit baed1ef.

Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
@antonydenyer antonydenyer force-pushed the bump_web3j branch 2 times, most recently from 2e2de4c to 5033283 Compare July 26, 2022 09:10
@diega diega marked this pull request as ready for review July 26, 2022 09:39
Use strictly to force the issue.

Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
@antonydenyer
Copy link
Member

@diega Had to force the dependency in the end! I think this is ready for review now.

@@ -50,7 +52,7 @@ public static BlockHeader createBlockHeader(
Bytes.fromHexString(block.getExtraData()),
null,
mixHash,
block.getNonce().longValue(),
new BigInteger(block.getNonceRaw().substring(2), 16).longValue(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diega
Copy link
Contributor Author

diega commented Jul 26, 2022

@antonydenyer nice trick! that way you'll be overriding the dependency added programmatically by the plugin, right?

@antonydenyer
Copy link
Member

antonydenyer commented Jul 26, 2022

@antonydenyer nice trick! that way you'll be overriding the dependency added programmatically by the plugin, right?

More like a sledgehammer that overrides everything - it will force everything to use '4.9.4', this could be a problem when you come to upgrade again. You could change the version in versions.gradle but it would have no affect. Perhaps we should use @freemanzMrojo approach as it has a little more finesse and may be easier for someone to spot. What do you think?

implementation('org.web3j:core') {
version {
strictly('4.9.4')
because('web3j plugin 4.9.4 hasn\'t been released yet')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
because('web3j plugin 4.9.4 hasn\'t been released yet')
because('Plugin version is 4.9.2 (latest), but we want it to use web3j libs version 4.9.4 ')

testImplementation 'org.web3j:quorum'
testImplementation('org.web3j:quorum') {
constraints {
implementation('org.web3j:core:4.9.4') { because 'quorum 4.9.4 hasn\'t been released yet' }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
implementation('org.web3j:core:4.9.4') { because 'quorum 4.9.4 hasn\'t been released yet' }
implementation('org.web3j:core:4.9.4') {
because 'Web3J/Quorum only has a 4.9.2 lib, but we want it to use web3j libs version 4.9.4 ')
}

@freemanzMrojo
Copy link
Contributor

Hi @antonydenyer @shemnon , hope you are doing well, and happy Friday! I was wondering if there is anything else to do in this PR, otherwise maybe you could merge it? I need it for #4086, thanks in advance!

@shemnon
Copy link
Contributor

shemnon commented Aug 1, 2022

I'm holding off for 22.7.0 to ship as this is not essential to the merge.

@shemnon shemnon merged commit 98dc2ac into hyperledger:main Aug 4, 2022
codyborn pushed a commit to codyborn/besu that referenced this pull request Aug 8, 2022
* Upgrade web3j dependencies to latest versions

Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Miguel Rojo <miguelangel.rojofernandez@mastercard.com>
Co-authored-by: Miguel Angel Rojo <freemanz1486@gmail.com>
codyborn pushed a commit to codyborn/besu that referenced this pull request Aug 8, 2022
* Upgrade web3j dependencies to latest versions

Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Miguel Rojo <miguelangel.rojofernandez@mastercard.com>
Co-authored-by: Miguel Angel Rojo <freemanz1486@gmail.com>
Signed-off-by: Cody Born <codyborn@outlook.com>
@diega diega deleted the bump_web3j branch August 29, 2022 15:38
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
* Upgrade web3j dependencies to latest versions

Signed-off-by: Diego López León <dieguitoll@gmail.com>
Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Antony Denyer <git@antonydenyer.co.uk>
Co-authored-by: Miguel Rojo <miguelangel.rojofernandez@mastercard.com>
Co-authored-by: Miguel Angel Rojo <freemanz1486@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants