Support codegen directly from Truffle json contract files #228

eepstein · 2017-11-09T05:21:32Z

See #226 .

This allows tighter integration of web3j tools with the Truffle suite so that there's more assurance code will work across both libraries.

Changes were made to master after this fork and before the commit of the Truffle import functionality.

codecov · 2017-11-09T05:26:42Z

Codecov Report

Merging #228 into master will decrease coverage by 0.5%.
The diff coverage is 62.29%.

@@             Coverage Diff              @@
##             master     #228      +/-   ##
============================================
- Coverage     77.32%   76.82%   -0.51%     
- Complexity     1534     1558      +24     
============================================
  Files           209      211       +2     
  Lines          5580     5731     +151     
  Branches        895      908      +13     
============================================
+ Hits           4315     4403      +88     
- Misses         1066     1119      +53     
- Partials        199      209      +10

Impacted Files	Coverage Δ	Complexity Δ
...onsole/src/main/java/org/web3j/console/Runner.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
utils/src/main/java/org/web3j/utils/Strings.java	`75% <100%> (-6.25%)`	`12 <5> (+2)`
...eb3j/codegen/SolidityFunctionWrapperGenerator.java	`59.61% <100%> (+1.28%)`	`4 <2> (-5)`	⬇️
core/src/main/java/org/web3j/tx/Contract.java	`74.33% <100%> (+1.69%)`	`36 <6> (+6)`	⬆️
.../protocol/core/methods/response/AbiDefinition.java	`100% <100%> (ø)`	`44 <4> (+7)`	⬆️
...ava/org/web3j/codegen/SolidityFunctionWrapper.java	`97.71% <100%> (+0.16%)`	`97 <4> (+4)`	⬆️
...j/codegen/TruffleJsonFunctionWrapperGenerator.java	`41.41% <41.41%> (ø)`	`5 <5> (?)`
...va/org/web3j/codegen/FunctionWrapperGenerator.java	`62.5% <62.5%> (ø)`	`7 <7> (?)`
...ore/src/main/java/org/web3j/utils/Observables.java	`80% <0%> (-8%)`	`9% <0%> (-2%)`
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8098c6a...9c435af. Read the comment docs.

eepstein · 2017-11-09T07:31:47Z

Seems unfair: My pull request is getting penalized because it picked up (and merged in) changes made by Conor, notably additions to the code that didn't include tests to cover that new code.

conor10 · 2017-11-09T10:06:39Z

utils/src/test/java/org/web3j/utils/StringsTest.java

        assertThat(toCsv(Collections.singletonList("a")), is("a"));
        assertThat(toCsv(Arrays.asList("a", "b", "c")), is("a, b, c"));
    }
-
-    @Test
-    public void testJoin() {


How come you removed this test?

That test was removed because I removed the Strings.join method since Java has a static String.join() method https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#join-java.lang.CharSequence-java.lang.CharSequence...- but again, if this is targetting JDK 1.6, then... whoops, undo.

@conor10 : do I need to put back in the hand-rolled Strings.join() utility method to support JDK 1.6 ?

Please leave the test in.

You can update the join util to be:

public static String join(List<String> src, String delimiter) { return src == null ? null : String.join(delimiter, src.toArray(new String[0])); }

Will do.

When I checked usages, the only place the code referenced that method is in the Strings utility class itself. The toCsv() method was calling it.

conor10 · 2017-11-09T10:07:33Z

utils/src/test/java/org/web3j/utils/StringsTest.java

@@ -19,16 +21,11 @@

    @Test
    public void testToCsv() {
-        assertThat(toCsv(Collections.<String>emptyList()), is(""));
+        assertThat(toCsv(Collections.emptyList()), is(""));


The parameterized value is so that this function works with Java 1.6 in the Android branch.

Android. Ah-ha. Maybe it's better then to change the gradle file to require Java v 1.6 ? (Unless I missed it, I may have only checked the top-level config and not the module's config.)

Do we want to change these lines in the root build.gradle file:

sourceCompatibility = 1.8 targetCompatibility = 1.8

conor10 · 2017-11-09T10:10:47Z

gradle/wrapper/gradle-wrapper.properties

 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-4.3-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-4.3-bin.zip


Did you consciously make these changes, or is your local gradle wrapper once downloaded different?

sorry about that. I thought I'd excluded the gradle wrapper folder entirely. Accident on my side if so.

Side note (and why I missed it): I always include it in my .ignore file since i view keeping the build tool in the repo as ... weird, rather I just have a gradle wrapper command to run when first pulling. Of course that means I've installed gradle, but then, most envs require you install the build tool. If some user is developing java they've already downloaded the JDK - and not just the JRE - and so they're 90% there.

conor10 · 2017-11-09T10:14:19Z

core/src/main/java/org/web3j/tx/Contract.java

+     * Subclasses should implement this method to return pre-existing addresses for deployed
+     * contracts.
+     *
+     * @param networkId the network id, for example "1" for the main-net, "3" for ropsten, etc.


You could put a link to the org.web3j.tx.ChainId class in this Javadoc

I actually searched the code base for ropsten, but I think I forgot to make it a case insensitive search. Hadn't seen ChainId. Curious why not just make that an enum?

Also, ChainId values are bytes, but the max value of a byte is 255 (unsigned). And the doc referenced actually shows 1337 as one of the recommended ids.

Shall we change ChainId to an enum and make the values int or, better, long ?

The reason for using a byte, is to support EIP-155, where you place a byte value in the raw transaction object - see here.

That being said further down the discussion in the original EIP it is mentioned that the v value when the transaction is RLP encoded shouldn't be limited to a single byte. So web3j could be enhanced to accomodate this. I've created a separate issue for this.

conor10

Very cool.

A few comments inline.

Please could you also update the docs with details too?

Specifically:

https://github.com/web3j/web3j#features
https://github.com/web3j/web3j#working-with-smart-contracts-with-java-smart-contract-wrappers
https://github.com/web3j/web3j/blob/master/docs/source/smart_contracts.rst#solidity-smart-contract-wrappers

…ort Android devs

eepstein · 2017-11-09T22:01:24Z

updating. i don't have the tools installed to build the docs, so here's hoping my edits are good

atching up with changes since this fork was made.

conor10 · 2017-11-10T18:17:04Z

A fair point - however, as the Java 6 version of toCsv is lengthier, I'd rather keep the two underlying functions.

…

On 10 November 2017 at 17:45, Ezra Epstein ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In utils/src/test/java/org/web3j/utils/StringsTest.java <#228 (comment)>: > assertThat(toCsv(Collections.singletonList("a")), is("a")); assertThat(toCsv(Arrays.asList("a", "b", "c")), is("a, b, c")); } - - @test - public void testJoin() { Will do. When I checked usages, the only place the code referenced that method is in the Strings utility class itself. The toCsv() method was calling it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#228 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACDHqiQox3pwEu1_LyVTSSQYPUU1HYuVks5s1IvDgaJpZM4QXc1D> .

-- Web: http://conorsvensson.com Phone: +44 7497 376 365 Skype: conor_svensson

conor10 · 2017-11-12T12:41:58Z

Thanks again @eepstein, this is a great enhancement.

Support codegen directly from Truffle json contract files

eepstein added 2 commits November 8, 2017 21:10

Support for generating Java source from Truffle .json contract files.

c503a2b

Merge branch 'master' of git://github.com/web3j/web3j

d46a77b

Changes were made to master after this fork and before the commit of the Truffle import functionality.

eepstein added 3 commits November 8, 2017 23:04

Appeasing the coverage police

03c7bb2

Don't reinvent wheels (esp not ones from java.lang.* !)

9bd2a86

formatting for readability

479b6ed

one more code path

8105eef

conor10 reviewed Nov 9, 2017

View reviewed changes

despite the current gradle settings, we should target JDK 1.6 to supp…

949cad5

…ort Android devs

eepstein and others added 10 commits November 9, 2017 14:02

updated docs for the new truffle command-line codegen option

c75b4d6

use the full gradle enchilada

8faba58

update main README.rst to talk about truffle json contract file support

0b0103c

needed more indenting to create sublist

b28bb68

needed more indenting to create sublist

976bfdd

indenting to create sublist

cd0774b

needed more indenting to create sublist

1f500b6

typo: webj -> web3j

a642ada

conor wants this method

25f5b54

Merge branch 'master' of git://github.com/web3j/web3j

f37f334

atching up with changes since this fork was made.

eepstein added 2 commits November 10, 2017 11:09

minimalist "toCsv" comma and space between values

f98ac59

Merge remote-tracking branch 'origin/master'

9c435af

conor10 merged commit 6e9c09b into hyperledger-web3j:master Nov 12, 2017

conor10 mentioned this pull request Nov 13, 2017

Support direct import of contracts compiled with Truffle #226

Closed

franz-see pushed a commit to franz-see/web3j that referenced this pull request Aug 3, 2018

Merge pull request hyperledger-web3j#228 from eepstein/master

ec2d3ca

Support codegen directly from Truffle json contract files

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support codegen directly from Truffle json contract files #228

Support codegen directly from Truffle json contract files #228

eepstein commented Nov 9, 2017

codecov bot commented Nov 9, 2017 •

edited

Loading

eepstein commented Nov 9, 2017

conor10 Nov 9, 2017

eepstein Nov 9, 2017

eepstein Nov 9, 2017

conor10 Nov 10, 2017

eepstein Nov 10, 2017

conor10 Nov 9, 2017

eepstein Nov 9, 2017

eepstein Nov 9, 2017

conor10 Nov 9, 2017

eepstein Nov 9, 2017

conor10 Nov 9, 2017

eepstein Nov 9, 2017 •

edited

Loading

conor10 Nov 10, 2017

conor10 left a comment

eepstein commented Nov 9, 2017

conor10 commented Nov 10, 2017 via email

conor10 commented Nov 12, 2017

Support codegen directly from Truffle json contract files #228

Support codegen directly from Truffle json contract files #228

Conversation

eepstein commented Nov 9, 2017

codecov bot commented Nov 9, 2017 • edited Loading

Codecov Report

eepstein commented Nov 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eepstein Nov 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conor10 left a comment

Choose a reason for hiding this comment

eepstein commented Nov 9, 2017

conor10 commented Nov 10, 2017 via email

conor10 commented Nov 12, 2017

codecov bot commented Nov 9, 2017 •

edited

Loading

eepstein Nov 9, 2017 •

edited

Loading