#98 - implemented encoding, estimate and deployment for contract creation and call #101

abdulowork · 2018-04-19T22:50:38Z

To implement #98 I separated transaction bytes computation into three separate purposes (contract deployment, contract function call, direct ether transaction). I added separate encoding for contract as EncodedContract and function as EncodedABIFunction. I added computation for contract address as ComputedContractAddress since ganache doesn't report contract address in the eth_getTransactionByHash call.

To solve #100 I added a missing check in SimpleRLP.

…and non contract cases

…elevant tests for newfound edge cases

0crat · 2018-04-19T22:51:35Z

Job #101 is now in scope, role is REV

driver733 · 2018-04-21T18:28:29Z

Example/Tests/RLP/RLPTests.swift

@@ -105,6 +105,98 @@ final class RLPTests: XCTestCase {
        )
    }

+    func test127() {


@Biboran Are double expects necessary?

driver733 · 2018-04-21T18:33:45Z

Example/Tests/TransactionBytes/EthContractCallBytesTests.swift

+
+    func testContractCallIsEncodedCorrectly() {
+        expect{
+            try EthContractCallBytes(


@Biboran Seven ctor parameters seems to be too much. What do you think about splitting the tested class into smaller ones?

@driver733 part of the requirement for the transaction signing is an ability to reproduce it by specifying every dependency. There are 7-8 mathematically independent dependencies for the transaction signing.

Do you see any way to retain that functionality and reduce the number of parameters?

@Biboran @driver733 lets accept that some functions require that number of parameters due to Ethereum architecture. We could code domain wrappers for them later after experimenting with real DApps. If it is ok then let's merge this.

driver733 · 2018-04-21T18:34:20Z

Example/Tests/TransactionBytes/EthContractCreationBytesTests.swift

+@testable import Web3Swift
+
+final class EthContractCreationBytesTests: XCTestCase {
+


@Biboran Seven ctor parameters seems to be too much. What do you think about splitting the tested class into smaller ones?

driver733 · 2018-04-21T18:37:07Z

Web3Swift/TransactionBytes/EthContractCallBytes.swift

+import Foundation
+
+//Bytes of a signed contract function call transaction
+public final class EthContractCallBytes: BytesScalar {


@Biboran Eight ctor parameters seems to be too much. What do you think about splitting the tested class into smaller ones?

driver733 · 2018-04-21T18:38:14Z

Web3Swift/TransactionBytes/EthContractCreationBytes.swift

+import Foundation
+
+//Bytes of a signed contract deployment transaction
+public final class EthContractCreationBytes: BytesScalar {


@Biboran Seven ctor parameters seems to be too much. What do you think about splitting the class into smaller ones?

driver733 · 2018-04-21T18:38:46Z

Web3Swift/TransactionBytes/EthDirectTransactionBytes.swift

+import Foundation
+
+//Bytes of a signed transaction to ethereum address
+public final class EthDirectTransactionBytes: BytesScalar {


@Biboran Seven ctor parameters seems to be too much. What do you think about splitting the class into smaller ones?

rockfridrich · 2018-04-24T12:54:46Z

Web3Swift/RemoteProcedure/VerifiedProcedure.swift

+
+//FIXME: This is a temporary workaround to allow throwing errors communicated by network. It is definitely possible to make this check perform only when the "result" was not found by reimplementing JSON.
+//Procedure that throws in case JSON RPC responded with an error
+public final class VerifiedProcedure: RemoteProcedure {


@Biboran we will need to use verified procedure everywhere. Is it better to create decorator for JSON?

@rockfridrich this is a temporary workaround as you can see is mentioned in the above fixme. I described the issue here: #58

rockfridrich · 2018-04-24T12:57:11Z

Web3Swift/TransactionBytes/EthContractCallBytes.swift

+        weiAmount: NumberScalar,
+        functionCall: BytesScalar
+    ) {
+        let senderAddress = CachedBytes(


@Biboran what is the purpose of CachedBytes?

@rockfridrich makes sure senderAddress is computed only once. Take a look at CachedBytes implementation.

rockfridrich · 2018-04-24T13:26:10Z

@Biboran there are a lot of objects like SimpleBytes, BigEndianNumber, CachedNumber and it is not clear how to use them.
We will need to have some wrappers later because it is madness to construct such objects

abdulowork · 2018-04-24T13:43:27Z

@rockfridrich the point of this ctor is to handle the complexity of signing a transaction. You only need to pass 5 vital parameters and everything else is handled inside. It might be a better idea to take it out and put into another object but I am not sure what it should be and I'll need help with that design.

If you will start with #96 and #99 you will gain an understanding of the design and you will be able tell me how it can be improved and where it's extremely vague.

0crat · 2018-04-24T13:50:32Z

This pull request #101 is assigned to @driver733 /z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @Biboran /z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewerWe should be aware that driver733 is on vacation; this ticket may be delayed

0crat · 2018-04-24T13:50:52Z

Manual assignment of issues is discouraged, see §19: -5 point(s) just awarded to @Biboran /z

0crat · 2018-04-29T14:36:36Z

@driver733 /z this job was assigned to you 5days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

driver733 · 2018-04-30T15:38:00Z

@0crat waiting

0crat · 2018-04-30T15:39:22Z

@0crat waiting (here)

@driver733 It's a code review job #101, can't put it on hold

0crat · 2018-05-06T13:53:45Z

The user @driver733 /z resigned from #101, please stop working. Reason for job resignation: It is older than 10 days, see §8

0crat · 2018-05-06T13:53:58Z

Resigned on delay, see §8: -15 point(s) just awarded to @driver733 /z

0crat · 2018-05-06T15:37:26Z

@Biboran /z everybody who has role REV are banned at this job; I won't be able to assign anyone automatically; consider assigning someone manually (as in §19), or invite more people (as in §51), or remove the job from the scope (as in §14)

…into 98

0crat · 2018-05-07T14:42:09Z

This pull request #101 is assigned to @driver733 /z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @Biboran /z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewerWe should be aware that driver733 is on vacation; this ticket may be delayed

0crat · 2018-05-07T14:42:28Z

Manual assignment of issues is discouraged, see §19: -5 point(s) just awarded to @Biboran /z

0crat · 2018-05-07T14:58:12Z

Order was finished: +15 point(s) just awarded to @driver733 /z

0crat · 2018-05-07T14:59:30Z

The job #101 is now out of scope

0crat · 2018-05-07T14:59:46Z

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @Biboran /z

abdulowork added 16 commits April 12, 2018 19:22

#98 - added separate contract and tuple to bytes coversion

8a89fc2

#98 - added different estimates and transaction signing for contract …

1e799ef

…and non contract cases

#98 - refactored transaction signing to allow a pure form

fc43511

#100 - added a required constraint on a single byte encoding, added r…

eec942f

…elevant tests for newfound edge cases

#100 - added a test case for a hypothetical 129th transaction

23209a5

#98 - implemented contract address computation

0337af4

#98 - added Tim to travis and buddybuild

75ed361

#98 - travis fix

5c78140

#98 - added contract call procedure and tests

28df3d5

#98 - added simple getter contract deployment and call testing

0bb6226

#98 - removed accidential code

b086fb4

#98 - added contract deployment signing testing

902df62

#98 - added verified procedure to assist debugging

545f04f

#98 - added contract call test

878dff1

#98 - fix

12cff4c

Merge branch 'develop' into 98

b516933

0crat added the scope label Apr 19, 2018

abdulowork requested review from rockfridrich and driver733 and removed request for rockfridrich April 19, 2018 22:51

driver733 suggested changes Apr 21, 2018

View reviewed changes

rockfridrich approved these changes Apr 24, 2018

View reviewed changes

abdulowork assigned driver733 Apr 24, 2018

driver733 approved these changes Apr 30, 2018

View reviewed changes

0crat unassigned driver733 May 6, 2018

abdulowork added 3 commits May 7, 2018 03:42

#98 - fix

9fed136

#98 - Merge branch '98' of https://github.com/BlockStoreApp/Web3Swift …

045bba5

…into 98

#98 - fixed incorrect values

c3be601

abdulowork assigned driver733 May 7, 2018

abdulowork merged commit 882d920 into develop May 7, 2018

0crat removed the scope label May 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#98 - implemented encoding, estimate and deployment for contract creation and call #101

#98 - implemented encoding, estimate and deployment for contract creation and call #101

abdulowork commented Apr 19, 2018

0crat commented Apr 19, 2018

driver733 Apr 21, 2018

driver733 Apr 21, 2018

abdulowork Apr 23, 2018 •

edited

Loading

rockfridrich Apr 30, 2018

driver733 Apr 21, 2018

driver733 Apr 21, 2018

driver733 Apr 21, 2018

driver733 Apr 21, 2018

rockfridrich Apr 24, 2018

abdulowork Apr 24, 2018

rockfridrich Apr 24, 2018

abdulowork Apr 24, 2018

rockfridrich commented Apr 24, 2018

abdulowork commented Apr 24, 2018 •

edited

Loading

0crat commented Apr 24, 2018

0crat commented Apr 24, 2018

0crat commented Apr 29, 2018

driver733 commented Apr 30, 2018

0crat commented Apr 30, 2018

0crat commented May 6, 2018

0crat commented May 6, 2018

0crat commented May 6, 2018

0crat commented May 7, 2018

0crat commented May 7, 2018

0crat commented May 7, 2018

0crat commented May 7, 2018

0crat commented May 7, 2018

		@testable import Web3Swift

		final class EthContractCreationBytesTests: XCTestCase {

#98 - implemented encoding, estimate and deployment for contract creation and call #101

#98 - implemented encoding, estimate and deployment for contract creation and call #101

Conversation

abdulowork commented Apr 19, 2018

0crat commented Apr 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abdulowork Apr 23, 2018 • edited Loading

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

rockfridrich commented Apr 24, 2018

abdulowork commented Apr 24, 2018 • edited Loading

0crat commented Apr 24, 2018

0crat commented Apr 24, 2018

0crat commented Apr 29, 2018

driver733 commented Apr 30, 2018

0crat commented Apr 30, 2018

0crat commented May 6, 2018

0crat commented May 6, 2018

0crat commented May 6, 2018

0crat commented May 7, 2018

0crat commented May 7, 2018

0crat commented May 7, 2018

0crat commented May 7, 2018

0crat commented May 7, 2018

abdulowork Apr 23, 2018 •

edited

Loading

abdulowork commented Apr 24, 2018 •

edited

Loading