-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement arkworks wrapper #1
base: master
Are you sure you want to change the base?
Conversation
Hi, when I compile the code on Mac, it shows the error:
After I change
To solve the problem, it shoub set |
Did you try that solution? I'm sorry , we have deliberately tested the build on linux and missed this problem with mac, and it seems that intended flag I have tried to set looking into it |
Set |
Hi, @Federico2014! Thank you for your feedback. I missed that |
@3for |
Excellent, it works now |
1)The lib in this pr does not work on MAC OS. So we fork and rebuild one in this repo.
where
|
Hello, @3for! Yes, it seems that a solution with
We can also deal with dynamic linking but it is probably not worth it if the solution above is valid. I'll try this solution and come back with the results. |
@AllFi I have tried as you said. It seems that it only works when compiled by myself. Others will show the errors. |
@Federico2014 could you please share the error? |
When I run the
I don't kwow why the above error includes the abosute path: ps: in order to show the error, I have changed the path |
I am not sure but it looks like it tries to use the old version of the library with dynamic linking. Could you please push a new version of the library in the https://github.com/Federico2014/zksnark-java-sdk-zkBob and try to build java-tron with p.s. It looks like you've already tried this solution even before I mentioned it. I guess the root of the current problem is gradle cache but if it is not the case I'll investigate further. |
|
Hi, @halibobo1205 , I've added built libraries for both x86 and arm on OSX. |
I re-download the https://github.com/zkBob/java-tron/, checkout the
|
Hello @3for! I've checked libraries again and everything seems to be correct. p.s. I guess |
I clean the cache by
Seems different from the lib:
|
@3for, sorry that the solution didn't work for you. i've built |
Hi @r0wdy1! It seems that the built library is already pushed in the https://github.com/Federico2014/zksnark-java-sdk-zkBob/tree/arkworks. Could you please check the |
@3for , we have pushed a new version
Java-tron works with this library , please test on your side |
Thanks so much for your help. We have passed tests now. @AllFi @r0wdy1 @halibobo1205 The libc version on our servers is 2.17, so I rebuilt the libzksnarkjni.so. Then the java-tron tests passed. |
Glad to be of help! Sorry for the inconvenience and thank you for your efforts in investigating the issues. I've updated the linux64 library in this branch too. What should be our next course of action? |
It works for me now. The tests passed. |
Great collaboration! As these PRs involve great performance improvements, maybe some upgrade compatibility and more tests should be considered, and it is better to merge them in a mandatory upgrade. The next mandatory upgrade of TRON is planned at the end of this year. Wondering if this date is suitable for you, but of course we can also make some additional attempts if needed. Moving these PRs or submitting news to the |
@tomatoishealthy , this repo ( zksnark-java-sdk) doesn't have In regards of the timeline of course we would be really interested in delivering this patches to production as early as possible, since it's pretty much the only thing that stops us from deploying on Tron network, so we will do everything possible on our end for that. |
@r0wdy1 @AllFi , Sry, my mistake. That's right, As this patch involves a relatively large performance improvement, if validators and fullnodes are not upgraded at the same time, it may cause a network fork. Therefore, the follow-up will also include some community coordination work and compatibility testing. Let us continue to push it to be online as soon as possible. |
So do you want both PRs moved to the corresponding tronprotocol organization repositories for better visibility?
Great, please let us know if we can help somehow. |
Yes.
Great! |
Hello, @tomatoishealthy! I've created new PRs in tronprotocol organization repositories: |
This PR introduces
LibarkworksWrapper
that uses rust implementation from arkworks to provide necessary functions forBN128Addition
,BN128Multiplication
, andBN128Pairing
precompiles.Caveats:
--allow-multiple-definition
flag was added to theCMakeLists.txt
to allow linking both Rust libraries. It is a bit risky. One potential solution is to introduce a wrapper Rust crate that depends on the previous two and compile it as a static library.libzksnarkjni.so
was rebuilt only for linux64