Updated JSON-RPC for Parity and added Geth calls #147
Conversation
| @@ -62,6 +63,43 @@ public void setVersion(int version) { | |||
| this.version = version; | |||
| } | |||
|
|
|||
| @Override | |||
| public int hashCode() { | |||
There was a problem hiding this comment.
With the hashCode and equals methods, please can you use the same implementations as per https://github.com/web3j/web3j/blob/master/src/main/java/org/web3j/protocol/core/methods/response/TransactionReceipt.java#L159
This is to ensure the code remains consistent throughout.
| } | ||
|
|
||
| @Override | ||
| public Request<?, PersonalUnlockAccount> personalUnlockAccount( |
There was a problem hiding this comment.
You shouldn't need to duplicate the logic in this second method - you should be able to use a single implementation, and override the implementation. Example here https://github.com/web3j/web3j/blob/master/src/main/java/org/web3j/protocol/parity/JsonRpc2_0Parity.java#L134
|
|
||
| @Override | ||
| public Request<?, BooleanResponse> parityChangePassword( | ||
| String accountId, String oldPass, String newPass) { |
There was a problem hiding this comment.
I would use oldPassword & newPassword here as the parameter names - no need to shorten them
|
|
||
| @Override | ||
| public Request<?, ParityDeriveAddress> parityDeriveAddressHash( | ||
| String accountId, String password, Map<String, Object> hashType, boolean toSave) { |
There was a problem hiding this comment.
You should create a class for hashType rather then using a map here. For example see https://github.com/web3j/web3j/blob/master/src/main/java/org/web3j/protocol/core/methods/request/Transaction.java
| @Override | ||
| public Request<?, ParityDeriveAddress> parityDeriveAddressIndex( | ||
| String accountId, String password, | ||
| List<Map<String, Object>> indexType, boolean toSave) { |
There was a problem hiding this comment.
HashType can be used here too
| public Request<?, PersonalUnlockAccount> personalUnlockAccount( | ||
| String accountId, String password) { | ||
| public Request<?, BooleanResponse> paritySetAccountName( | ||
| String accountId, String newAccountName) { |
There was a problem hiding this comment.
These parameters should be "address" and "name".
| } | ||
|
|
||
| @Override | ||
| public Request<?, EthSendTransaction> personalConfirmRequest( | ||
| public Request<?, EthSendTransaction> signerConfirmRequest( |
There was a problem hiding this comment.
It would be better to use a custom type here instead of the existing Transaction type which has a lot of unused fields for this method.
There was a problem hiding this comment.
On second thoughts - you can get rid of the signer module methods - given its only a subset of them anyway. I'd question how useful they are for web3j users.
|
|
||
| PersonalSign personalSign = deserialiseResponse(PersonalSign.class); | ||
| //CHECKSTYLE:OFF | ||
| assertThat(personalSign.getSignedMessage(),is("0xf1aabd691c887ee5c98af871239534f194a51fdeb801b1601d77c45afa74dae67ddd81aa5bb8a54b7974ef5be10b55a8535b040883501f76d14cb74e05e5635d1c")); |
There was a problem hiding this comment.
the "is(...)" should be on a new line
|
I've used netbeans to autogenerate equals and hashcode, hence the discrepancy. |
Codecov Report
@@ Coverage Diff @@
## master #147 +/- ##
============================================
- Coverage 80.12% 78.03% -2.09%
- Complexity 1199 1228 +29
============================================
Files 171 179 +8
Lines 4302 4335 +33
Branches 632 656 +24
============================================
- Hits 3447 3383 -64
- Misses 712 778 +66
- Partials 143 174 +31
Continue to review full report at Codecov.
|
|
Great work! |
Updated JSON-RPC for Parity and added Geth calls
Response to changes of Parity namespace as detailed in this PR.
Implemented parity_accounts Module.
Implemented Geth personal Module.
Implemented tests for all Response/Request.