-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 a condition on the duplicated event names #1872
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good workaround for now. Please add also a unit test to can include it in today release.
codegen/src/main/java/org/web3j/codegen/SolidityFunctionWrapper.java
Outdated
Show resolved
Hide resolved
codegen/src/main/java/org/web3j/codegen/SolidityFunctionWrapper.java
Outdated
Show resolved
Hide resolved
Added the unit test. Not sure if that's the proper way of doing it, please check. I had to put the buildFunctionDefinitions package-private since it was not accessible inside tests. I also removed the usage of changeEventsName() in buildFuncNameConstants(), l.742 since the function operate just on TYPE_FUNCTION objects (correct me if I'm wrong). |
Apparently my Unit Test is not working. On my local machine, anyway, it correctly builds and passes tests. |
+ " java.util.Arrays.<org.web3j.abi.TypeReference<?>>asList(new org.web3j.abi.TypeReference<org.web3j.abi.datatypes.Utf8String>() {}, new org.web3j.abi.TypeReference<org.web3j.abi.datatypes.Bool>() {}));\n" | ||
+ " ;\n" | ||
+ "\n" | ||
+ " public static final org.web3j.abi.datatypes.Event EVENTNAME1_EVENT = new org.web3j.abi.datatypes.Event(\"eventName1\", \n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Event name shouldn't be modified new org.web3j.abi.datatypes.Event(\"eventName1\"...
will fail in request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please give me some more details about that?
Do you mean it is not possible to use the abiDef.setName() function on TYPE_EVENTS objects?
Thanks
I won't add it to this release as it affects the wrapper and has to be carefully tested also by us. |
@gtebrean can you please give me some more information if you have them? With the new commit the eventName passed in the constructor should be the same. In my local env. the tests passed, but apparently there's still something which I'm doing wrongly since the checks failed. |
jobs are complaining that: It fails to create the wrapper, make sure you have latest changes from master. |
My branch when I commited was up-to-date with master. The method org.web3j.codegen.SolidityFunctionWrapperTest > testBuildFunctionDuplicatedEventNames() is the unit test I created my self for this PR, but in my local env is passing the test, whereas in the jobs it's not. Which could be the cause? Error logs aren't really useful. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
What does this PR do?
Add a condition on the name of the events of the java wrapper
Where should the reviewer start?
There is just a one file.
Why is it needed?
Based on Issue #1781, for contracts which have 'overloaded' events (same name, but different signatures) the Java Wrapper creates those events as different instances of the Event class, but with the same variable name, thus resulting in an error. This PR adds the
changeEventsName()
method which uses an HashMap to count the occurences of an event name, and for those with more than one occurence, it will change the name putting a number at its end, e.g.