Deposit contract handling using Web3J via JSON-RPC#170
Deposit contract handling using Web3J via JSON-RPC#170
Conversation
mkalinin
left a comment
mkalinin
left a comment
There was a problem hiding this comment.
Great job is done! I left a few requests for improvement.
|
|
||
| import java.util.Scanner; | ||
|
|
||
| public class ContractSource { |
There was a problem hiding this comment.
A thought of making this class a singletone. This is rather a cosmetic change than something functional though.
|
|
||
| dependency "org.apache.logging.log4j:log4j-api:${log4j2Version}" | ||
| dependency "org.apache.logging.log4j:log4j-core:${log4j2Version}" | ||
| dependency "org.slf4j:slf4j-simple:1.7.28" |
There was a problem hiding this comment.
Why do we need slf4j if we already have log4j2?
|
|
||
| RUN apk add --no-cache make gcc musl-dev linux-headers git | ||
|
|
||
| ARG branch=master |
There was a problem hiding this comment.
I think it would be better to stick with some tagged version in order to prevent unnecessary code and/or dependency updates on the geth side that could break docker container or the test itself.
| * | ||
| * <p>Generated with web3j version 4.3.2-SNAPSHOT. | ||
| * | ||
| * <p>Generated with following input: org.web3j.codegen.SolidityFunctionWrapperGenerator.java -b /home/work/projects/beacon-chain-java/pow/core/src/main/resources/org/ethereum/beacon/pow/ContractBin.bin -a /home/work/projects/beacon-chain-java/pow/core/src/main/resources/org/ethereum/beacon/pow/ContractAbi.json -o /home/work/projects/beacon-chain-java/pow/web3j/src/main/java -p org.ethereum.beacon.pow.contract </p> |
There was a problem hiding this comment.
We might want to reuse this script later on. Would you mind to include it into a codebase and leave a reference to it here?
| web3RequestExecutor.executeOnSyncDone( | ||
| () -> { | ||
| processConfirmedBlocks(); | ||
| onEthereumUpdated(); |
There was a problem hiding this comment.
onEthereumUpdated() in its turn calls to processConfirmedBlocks(). Is that by design?
| block -> { | ||
| onEthereumUpdated(); | ||
| }) | ||
| .subscribe(); |
There was a problem hiding this comment.
Does that mean that we're subscribed to blocks from very beginning before sync is done? We might want to get this subscription right on sync done event. Is that doable with web3j? Probable obstacle that I can see so far is that sync done event is not always emitted. For example, would it be emitted if sync was done before we get subscribed?
| web3j | ||
| .ethGetBlockByNumber(DefaultBlockParameter.valueOf(BigInteger.valueOf(number)), false) | ||
| .sendAsync() | ||
| .join(); |
There was a problem hiding this comment.
I am thinking of processing blocks and transaction receipts in an asynchronous fashion. We probably can get a block flow out of web3j, then add a processor that filters out blocks with usage of bloom filter, then yet another one which requests for tx receipts of valuable blocks. I guess we can created a stream upon a set of futures that are requesting for receipts. And at the end parse deposit data from tx receipts upon their arrival.
This scheme has at least one flaw, deposit events might be processed in the wrong order but I don't think that there will be a big mess. So we could probably have a buffer that could be drained when there is a deposit standing next to last processed one.
| BigInteger nonce = txCountFut.join().getTransactionCount(); | ||
| BigInteger value = | ||
| Convert.toWei(amount.longValue() + "", Convert.Unit.GWEI).toBigInteger(); | ||
| BigInteger gasLimit = BigInteger.valueOf(2_000_000); |
There was a problem hiding this comment.
Let's create a constant for this magic number.
|
|
||
| @Override | ||
| public CompletableFuture<BytesValue> signTransaction( | ||
| BytesValue unsignedTransaction, BytesValue eth1PrivKey) { |
There was a problem hiding this comment.
We might want to avoid private key management and dedicate it to geth or parity. Any thoughts on that?
| ``` | ||
| Running container: | ||
| ```shell script | ||
| docker run -d -p 8545:8545 -p 30303:30303 <image id> |
There was a problem hiding this comment.
It would be better to give this image a name: docker build -t name && docker run -d name.
|
|
||
| @Before | ||
| public void setUp() throws Exception { | ||
| this.web3j = Admin.build(new HttpService()); |
There was a problem hiding this comment.
Since RPC URL may vary from one env to another it would be better if it's explicitly passed to web3j.
2 participants
pow:web3jimplementation plus some fixes topow:ethereumj