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

Add function to link binary with reference libraries in wrappers contract deployment #1988

Merged
merged 14 commits into from
Mar 14, 2024

Conversation

tonykwok1992
Copy link
Contributor

What does this PR do?

Add function to link binary with reference libraries in wrappers contract deployment

Where should the reviewer start?

All files

Why is it needed?

Currently there is no way to deploy contract that have library unlinked, Adding this functionality so that we can link the bytecode library placeholder with library address before deployment

@gtebrean
Copy link
Contributor

gtebrean commented Jan 4, 2024

@tonykwok1992 could you please add here a solidity smart contract example for this case?

@cybernagl
Copy link

Let me try and give the example, @gtebrean

Assume you have a solidity library, like so...

library SomeLib {
    function someUsefulLibraryFunction() public pure returns (string memory) {
        return "...";
    }
}

...and a contract that makes use of this library, like so...

import "./SomeLib.sol";

contract SomeContract {
    function someFunction() public view returns (string memory message) {
        message = SomeLib.someUsefulLibraryFunction();
    }
}

To be able to deploy SomeContract, one must link it to SomeLib.

In a migrate script for Truffle you might do it like this:

const library = artifacts.require("SomeLib");
const contract = artifacts.require("SomeContract");
module.exports = async function (deployer, networks, accounts) {
    await deployer.deploy(library);
    const libraryInstance = await errors.deployed();
    if (!libraryInstance.address) throw "Failed to deploy library";

    /* this is not possible from web3j right now */
    await deployer.link(errors, [contract]);

    await deployer.deploy(contract);
    contractInstance = await contract.deployed();
}

@tonykwok1992
Copy link
Contributor Author

tonykwok1992 commented Feb 27, 2024

Thanks for @cybernagl 's help to provide the solidity example. It is for the corresponding functionality in hardhat:
https://hardhat.org/hardhat-runner/plugins/nomicfoundation-hardhat-ethers#library-linking

also some inspiration from etherjs ethers-io/ethers.js#195

Currently this feature is not available in web3j

Another thought on where to place this change is to have it in deploy function generated, although it requires a bit more change and currently there are already 4 deploy functions generated and adding these to deploy function would result in more permutation of deploy

@NickSneo
Copy link
Contributor

NickSneo commented Mar 12, 2024

@tonykwok1992 The build is failing in local, these tests are failing -

SolidityFunctionWrapperGeneratorTest. testArraysInStructCompareJavaFileTest()
SolidityFunctionWrapperGeneratorTest. testEventParametersNoNamedCompareJavaFile()
SolidityFunctionWrapperGeneratorTest. testStructOnlyInArrayCompareJavaFile()
JavaClassGeneratorTest. initializationError
JavaParserTest. initializationError
MethodParserTest. initializationError
FunParserTest. initializationError
KotlinClassGeneratorTest. initializationError
KotlinParserTest. initializationError

Also plz rebase it to master and fix the failing tests, Thanks!

Copy link
Contributor

@NickSneo NickSneo left a comment

Choose a reason for hiding this comment

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

Do a spotlessApply, rebase to master and Fix the failing tests

@tonykwok1992
Copy link
Contributor Author

Do a spotlessApply, rebase to master and Fix the failing tests

All done

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 69.21%. Comparing base (a1f47f9) to head (c495a39).

Files Patch % Lines
core/src/main/java/org/web3j/tx/Contract.java 4.76% 20 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1988      +/-   ##
============================================
- Coverage     69.22%   69.21%   -0.01%     
- Complexity     3116     3123       +7     
============================================
  Files           493      493              
  Lines         13093    13142      +49     
  Branches       1692     1695       +3     
============================================
+ Hits           9063     9096      +33     
- Misses         3538     3556      +18     
+ Partials        492      490       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tonykwok1992
Copy link
Contributor Author

Made an enhancement to introduce librariesLinkedBinary so that we can maintain the original static final BINARY untouched, and we do not need to generate link libraries code when wrapper is generated without bin

Copy link
Contributor

@NickSneo NickSneo left a comment

Choose a reason for hiding this comment

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

LGTM

@tonykwok1992
Copy link
Contributor Author

rebased

@gtebrean gtebrean closed this Mar 14, 2024
@gtebrean gtebrean reopened this Mar 14, 2024
@gtebrean gtebrean merged commit 13015e5 into hyperledger:master Mar 14, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants