working anchor prototype #13

pstehlik · 2018-02-07T15:01:53Z

No description provided.

…re acceptance of data

…witness2anchor

lucasvo · 2018-02-19T21:02:58Z

README.md


-and then copy the `witness_contract.go` file to `centrifuge/witness/`. You will also need to modify the file to add the following imports:
+and then copy the `ethereum_anchor_registry_contract.go` file to `centrifuge/anchor/`. You will also need to modify the file to add the following imports:


Why not just leave the go files in the contract repository and list them as dependencies here?

import "github.com/CentrifugeInc/centrifuge-ethereum-contracts/centrifuge/witness"

I don't think the contracts repo should hold the representation of all the possible client code. this should IMO be done by the implementing client instead.
or maybe by an intermediary project

Are we planning on committing the ABI? That fits into the same category IMO.

ABI is a bit in-between as it is somewhat native to the project. But eventually, I wouldn't commit the ABI either in the future. Just the code and then have a "release project" somewhere else to keep artifacts that are required for downstream projects.

lucasvo · 2018-02-19T21:09:19Z

centrifuge/anchor/ethereum_anchor_registry.go

+	watchOpts := &bind.WatchOpts{}
+	watchOpts.Context = ethereum.DefaultWaitForTransactionMiningContext()
+
+	sinkNo := make(chan *EthereumAnchorRegistryContractAnchorRegistered, 100)


What does "sinkNo" mean?

lucasvo · 2018-02-19T21:12:50Z

centrifuge/anchor/ethereum_anchor_registry.go

@@ -38,59 +39,65 @@ func (ethRegistry *EthereumAnchorRegistry) RegisterAnchor(anchor *Anchor) (Ancho
 	copy(bAnchorId[:], anchor.anchorID[:32])
 	var schemaVersion = big.NewInt(int64(anchor.schemaVersion))

+	//listen to this particular anchor being mined/event is triggered


Do we really wait for it to be mined here? This could be an hour or more. Would we really want to rely on in channel memory for tracking which anchors still need to be mined? There could be quite a lot of these tx and restarting the process would lead to loosing all of them right?

lucasvo · 2018-02-28T12:51:36Z

README.md


-and then copy the `witness_contract.go` file to `centrifuge/witness/`. You will also need to modify the file to add the following imports:
+and then copy the `ethereum_anchor_registry_contract.go` file to `centrifuge/anchor/`. You will also need to modify the file to add the following imports:


Are we planning on committing the ABI? That fits into the same category IMO.

lucasvo · 2018-02-28T12:57:29Z

centrifuge/anchor/anchor.go

+package anchor
+
+type Anchor struct {
+	AnchorID      string


Why are you using a string here? An Anchor ID (and root hash) are not necessarily valid ascii strings. So representing those as a string doesn't give us any readable values unless we make the extra effort to encode them in base64 which is what we are doing when exposing them as in the REST api. I would not encode it in base64 as long as it doesn't need to be human readable.

I would make them all 32 char strings for upper level APIs. We don't really gain much from dealing just with bytes there and it is less intiutive in upstream APIs to deal with bytes (logging, debugging, etc).

But a 32 char string is not human readable either, because not every 8bit value is a valid ascii character in fact only 64 are human readable. You will end up with something that looks like garbage in your logs if you log a 256bit value converted to an ascii string (that's why people use base64 encoding). In the public api we already automatically convert a 256bit value to base64 strings and it's quite easy to make our logging do the same.

You could have a custom byte type that just implements a different to string method that would print base64 encoding or you actually need to convert it to base64. I don't think it is a good idea, in many places (e.g. in libp2p) byte values are expected anyway.

Here's an example that shows the issue of not converting to base64 and how we could still use raw byte values without actually using base64.

package main import "encoding/base64" import "fmt" type base64Byte []byte func (b base64Byte) String() string { return base64.StdEncoding.EncodeToString(b) } func main() { by := []byte{1,29, 200, 29, 210, 55, 102} by64 := base64Byte(by) fmt.Println("Raw bytes:", by) fmt.Println("String of bytes:", string(by)) fmt.Println("Base64 byte:", by64) } // Output Raw bytes: [1 29 200 29 210 55 102] String of bytes: ��7f Base64 byte: AR3IHdI3Zg==

See https://play.golang.org/p/ghL7iCk5Dk8

lucasvo · 2018-02-28T13:00:35Z

centrifuge/anchor/ethereum_anchor_registry.go

+package anchor
+
+import (
+	//"github.com/CentrifugeInc/go-centrifuge/centrifuge/ethereum"


lucasvo · 2018-02-28T13:05:50Z

centrifuge/anchor/ethereum_anchor_registry.go

+	}
+}
+
+func getAnchorContract() (anchorContract *EthereumAnchorRegistryContract, err error) {


Godoc style guide says that function descriptions should start with the full function name and be right outside the function. There are a few other spots as well where this is switched around.

Also it seems like it's not displaying anything :)

// getAnchorContract instantiates the contract and display its name func getAnchorContract() (anchorContract *EthereumAnchorRegistryContract, err error) {

lucasvo · 2018-02-28T13:10:14Z

centrifuge/anchor/ethereum_anchor_registry_test.go

+
+import (
+	"testing"
+	"github.com/stretchr/testify/assert"


I haven't used this library yet. (I did all my assertions manually) Did you put much thought into which "assertion" library we should use or did you just pick the first option you came across?

Might make sense to spend half an hour on looking at a few and then make a decision for one. I'd refactor my own code to use that library then. If you are saying that testify.assert is the best option, I'll go ahead and add it to my tests as well.

Yep, it is the one I have seen used most often. We should use this to build up our testing framework for all our test cases.

For simple cases testify is good. However, I also created #24 to look into another framework that I found quite interesting when I did some research.

mikiquantum · 2018-03-01T18:10:45Z

centrifuge/anchor/ethereum_anchor_registry.go

+
+// Setting up the listened for the "AnchorRegistered" event to notify the upstream code about successful mining/creation
+// of the anchor.
+func setUpRegistrationEventListener(ethRegistryContract WatchAnchorRegistered, anchorToBeRegistered *Anchor, confirmations chan<- *Anchor) (err error) {


I like the asynch nature of this implementation, just a couple of comments:

As we would have multiple types of ethereum "objects" would be good to try to abstract the event listener out of the anchoring domain. As we have an immediate use case with Identities, where we would like to have the same listener approach for any type. I would like to do that, as part of the identity work I am about to start.

That being said, we will need to redesign this code anyways as we should be able to store pending requests so the flow works even if the node goes down.

As soon as we merge this in, I would refactor it to be a bit more abstract to process as well identities, or other types. In addition I will create a task to make this resilient to restarts.

mikiquantum · 2018-03-01T18:41:24Z

scripts/test-ethereum/genesis.json

+    "eip155Block": 0,
+    "eip158Block": 0
+  },
+  "difficulty": "x200",


Will this need to be "0x200" instead ?

mikiquantum · 2018-03-01T19:33:25Z

I was trying to run the integration tests by running:
go test ./centrifuge/anchor/anchor_registry_integration_test.go --ethereum -v
wrapped in a script that binds env vars needed:

export CENT_ETHEREUM_GETHIPC=$HOME/.centrifuge/geth_test_network/geth.ipc
export CENT_ETHEREUM_GASLIMIT=4712388
export CENT_ETHEREUM_GASPRICE=40000
export CENT_ETHEREUM_ACCOUNTS_MAIN_PASSWORD=ZhXfpAc#vHu4JTELA
export CENT_ETHEREUM_ACCOUNTS_MAIN_KEY='{"address":"838f7dca284eb69a9c489fe09c31cff37defdeca","crypto":{"cipher":"aes-128-ctr","ciphertext":"b16312912c00712f02b43ed3cdd3b3172195329415527f7ee218656888aa5d92","cipherparams":{"iv":"19494c514fae0e4d83d9a7e464e89e29"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"e9b7cf9b55eab4a54f6f6f5af98ca6add2ca49147d37f99a5fa26a89e9003517"},"mac":"04805d48727a24cc3ee2ac2198f7fd5be269e52ff105c125cd10b614ce0d856d"},"id":"cd3800bc-c85d-457b-925b-09d809d6b06e","version":3}'
export CENT_ANCHOR_ETHEREUM_ANCHORREGISTRYADDRESS="0x995ef27e64cb9ef07eb6f9d255a3951ef20416fd"

I have running the local geth by executing prepare.sh and run.sh

Tests fail waiting for the transaction to finish:

=== RUN   TestRegisterAsAnchor_Integration
2018/03/01 14:30:33 Sent off the anchor [id: 5d379253d793682d177fb743bf328b9febb6136191778fea3e3263dffe9a039f, hash: c8c18610c86093f09afeaede4718cbc28cafea8464336a7125b42c8a0eacd609, SchemaVersion:1] to registry. Ethereum transaction hash [bca28f9d83b7bb75d82636fb38b0696fcfc2519580d5e8da44a77eb5ea07cc3b]
2018/03/01 14:30:33 Transfer pending: 0xbca28f9d83b7bb75d82636fb38b0696fcfc2519580d5e8da44a77eb5ea07cc3b
2018/03/01 14:31:01 Context closed before receiving AnchorRegistered event for anchor ID: ]7�Sדh-��C�2���a�w��>2c����, RootHash: ����`�����G����d3jq%�,���
exit status 1
FAIL	command-line-arguments	31.513s

Any ideas, is there something off with my setup?

When attaching to the ipc I see:

eth.getTransactionReceipt("0xbca28f9d83b7bb75d82636fb38b0696fcfc2519580d5e8da44a77eb5ea07cc3b")
{
  blockHash: "0x61ffccf0e3eb2c1f68f6baf1617a18b0056501a484edab92fb38b0b184dc4bca",
  blockNumber: 2107,
  contractAddress: null,
  cumulativeGasUsed: 25816,
  from: "0x838f7dca284eb69a9c489fe09c31cff37defdeca",
  gasUsed: 25816,
  logs: [],
  logsBloom: "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
  root: "0x09470e8e59db8d46f5e82c7c57b5deb43cda1ed6017be43714be8cade2ad3eaf",
  to: "0x995ef27e64cb9ef07eb6f9d255a3951ef20416fd",
  transactionHash: "0xbca28f9d83b7bb75d82636fb38b0696fcfc2519580d5e8da44a77eb5ea07cc3b",
  transactionIndex: 0
}

seems to be ok

… mod: filter anchor registered event by "from" address

…witness2anchor

pstehlik added 9 commits February 7, 2018 10:00

working anchor prototype

05dbce5

working anchor prototype

10fba04

testing out how to get the correct transaction ID from client to assu…

7e6c24d

…re acceptance of data

Merge branch 'master' of github.com:centrifugeinc/go-centrifuge into …

840d756

…witness2anchor

working 'integration test' setup with bindings from 1.7.3

2071263

adding geth tooling scripts

83097b8

upgrading to go-ethereum 1.8 and testing

fc6b088

adding go172 launch script as workaround for trufflesuite/truffle#721

ae105ac

working 'integration test' to subscribe to logs/events

ebdfb31

lucasvo reviewed Feb 19, 2018

View reviewed changes

pstehlik added 12 commits February 23, 2018 16:01

working version with channel pass-through

e24a27e

allowing to run eth commands from any directory

ef48207

faster block time for geth

d1a927f

easier unit testing for anchoring. adding string tools.

945f9a4

fix witness code to run tests

c88f93c

more unit testing

5aa4ce6

adding tests for tools package

a8191e6

configuration for get_client coming from viper again

b7caf83

anchor registry configurable through env vars

74fe6b3

support multiple eth accounts, start concurrency handling in eth client

cfde4ae

make eth client init thread safe

41ad861

clean up anchor creation safety, start concurrency handling

0c44d54

lucasvo reviewed Feb 28, 2018

View reviewed changes

mikiquantum reviewed Mar 1, 2018

View reviewed changes

pstehlik added 2 commits March 2, 2018 20:16

add: comments, contextWaitTimeout as configuraton. fix: genesis file,…

edacd87

… mod: filter anchor registered event by "from" address

Merge branch 'master' of github.com:centrifugeinc/go-centrifuge into …

236194c

…witness2anchor

pstehlik merged commit 1ba83f6 into centrifuge:master Mar 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

working anchor prototype #13

working anchor prototype #13

pstehlik commented Feb 7, 2018

lucasvo Feb 19, 2018

pstehlik Feb 28, 2018

lucasvo Feb 28, 2018

pstehlik Mar 3, 2018

lucasvo Feb 19, 2018

lucasvo Feb 19, 2018

lucasvo Feb 28, 2018

lucasvo Feb 28, 2018

pstehlik Mar 3, 2018

lucasvo Mar 3, 2018

lucasvo Mar 3, 2018

lucasvo Mar 3, 2018

lucasvo Feb 28, 2018

lucasvo Feb 28, 2018

lucasvo Feb 28, 2018

mikiquantum Mar 1, 2018

pstehlik Mar 3, 2018

mikiquantum Mar 1, 2018 •

edited

Loading

mikiquantum Mar 1, 2018

mikiquantum commented Mar 1, 2018 •

edited

Loading


		and then copy the `witness_contract.go` file to `centrifuge/witness/`. You will also need to modify the file to add the following imports:
		and then copy the `ethereum_anchor_registry_contract.go` file to `centrifuge/anchor/`. You will also need to modify the file to add the following imports:

working anchor prototype #13

working anchor prototype #13

Conversation

pstehlik commented Feb 7, 2018

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

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

mikiquantum Mar 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikiquantum commented Mar 1, 2018 • edited Loading

mikiquantum Mar 1, 2018 •

edited

Loading

mikiquantum commented Mar 1, 2018 •

edited

Loading