Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Baseledger p2p client #16

Open
wants to merge 15 commits into
base: dev2
Choose a base branch
from
Open

Baseledger p2p client #16

wants to merge 15 commits into from

Conversation

skosito
Copy link
Contributor

@skosito skosito commented Aug 12, 2021

I think this is ready for first pass, i left couple of comments with question.
Provide-go PR for this: provideplatform/provide-go#7

@skosito
Copy link
Contributor Author

skosito commented Aug 12, 2021

far from ready to be merged, opened PR to easily review diffs as we go

@@ -333,6 +333,114 @@ func EthereumNetworkStatsDataSourceFactory(network *network.Network) *NetworkSta
}
}

// TMP struct, just for testing
// format is still unknown
type BaseledgerSubscriptionResponse struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all types are moved to provide-go PR: provideplatform/provide-go#7
for simplicity let's merge that one first and i will update version here and clean up - let me know if there is easier process, maybe local go package somehow?

} else {
if result, ok := response.Result["data"].(map[string]interface{}); ok {
if resultJSON, err := json.Marshal(result); err == nil {
header := &BaseledgerBlockHeader{}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unmarshalling full response led to time parse error, like this it is ok, just unmarshalling header at the end
probably format will change with our custom events, so i would leave it like this for now, i think it is working fine
will do more testing

@@ -198,6 +198,12 @@ func consumeShuttleContractDeployedMsg(msg *stan.Msg) {
func(c *contract.Contract, tokenType, name string, decimals *big.Int, symbol string) (createdToken bool, tokenID uuid.UUID, errs []*provide.Error) {
common.Log.Debugf("resolved %s token: %s (%v decimals); symbol: %s", *network.Name, name, decimals, symbol)

var address *string
if network.IsEthereumNetwork() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a quick fix, is there a already better convention in code for situations like this? it seems until now all networks were ethereum type... maybe we can start some strategy pattern or something similar for contracts addresses? or is this ok for now? same question below where i had to do the same

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally it would just fit into the API provider pattern that we are now using throughout the Network impl.

@skosito skosito changed the title [WIP] Baseledger p2p client Baseledger p2p client Aug 20, 2021
@skosito skosito requested a review from kthomas August 20, 2021 14:05
// { "jsonrpc": "2.0", "method": "subscribe", "params": ["tm.event='NewBlock'"], "id": 1 }
payload := map[string]interface{}{
"method": "subscribe",
"params": []string{"tm.event='NewBlockHeader'"},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 NewBlockHeader

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need different event? i was using this one, from tendermint

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is good.

network/node.go Outdated
@@ -749,6 +749,8 @@ func (n *Node) P2PAPIClient() (p2p.API, error) {
apiClient = p2p.InitParityP2PProvider(rpcURL, n.NetworkID.String(), n.Network)
case p2p.ProviderQuorum:
apiClient = p2p.InitQuorumP2PProvider(rpcURL, n.NetworkID.String(), n.Network)
case p2p.ProviderBaseledger:
apiClient = p2p.InitBaseledgerP2PProvider(rpcURL, n.NetworkID.String(), n.Network)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe put this in alphabetical order? Nit.

tx/tx.go Outdated
@@ -1414,7 +1420,7 @@ func (t *Transaction) handleTxTraces(
network *network.Network,
signerAddress string,
traces *provideapi.TxTrace,
receipt *provideapi.TxReceipt,
receipt *p2p.TxReceipt,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be updated here with new provide-go stuff, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kthomas what is the process for go packages? will you release new one or? please let me know and i will update this PR, i think that is the only thing left to do here and then clean up

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your changes were merged into provide-go... just go get github.com/provideplatform/provide-go@4728f8d567a156fd8beef2bf7153ffb306512642 and make mod...

Copy link
Contributor

@kthomas kthomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good.

Would love to see logs attached to the PR.

Also, did the test API method that was being used for convenience get removed?

@skosito
Copy link
Contributor Author

skosito commented Aug 20, 2021

@kthomas
thanks for review
i removed that API handler
regarding logs, which logs you refer to, statsdeamon or?

@kthomas
Copy link
Contributor

kthomas commented Aug 20, 2021

@kthomas
thanks for review
i removed that API handler
regarding logs, which logs you refer to, statsdeamon or?

Logs of this running- yes, statsdaemon would be great. Would be good to see subscription to the log emissions too.

err := json.Unmarshal(resultJSON, header)
if err != nil {
common.Log.Warningf("Failed to stringify result JSON in otherwise valid message received on network stats websocket: %s; %s", response, err.Error())
} else if header != nil {
common.Log.Debugf("Block header received %v\n", result)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kthomas log example:

time="2021-08-22T19:44:01+02:00" level=debug msg="Received 3879-byte message on network stats websocket for network: peachtree baseledger"
time="2021-08-22T19:44:01+02:00" level=debug msg="Block header received map[type:tendermint/event/NewBlockHeader value:map[header:map[app_hash: chain_id:peachtree consensus_hash:048091BC7DDC283F77BFBF91D73C44DA58C3DF8A9CBC867405D8B7F3DAADA22F data_hash:E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855 evidence_hash:E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855 height:46852 last_block_id:map[hash:3D96FB1BA50A49B2AD84E4287C65E7E34B576382FC9E8279D0AFD56F3949DAC3 parts:map[hash:6AD6BCD19CF3BFB788134A2CA9ACEBFC831A34E1CC275C098BA9337AA5478EB4 total:1]] last_commit_hash:DFCC3C40D2B410F93529D17DED45D69B06849EB18BA10171523C8577DB27AF94 last_results_hash:E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855 next_validators_hash:65EC6D39C0D6053F9D916033B24139BAEA7A5392ACE68C233ACFDF8401ED5730 proposer_address:E0F0CE7A37BE16EDE67F70831D5608C5EA6E8540 time:2021-08-22T17:43:52.995586714Z validators_hash:65EC6D39C0D6053F9D916033B24139BAEA7A5392ACE68C233ACFDF8401ED5730 version:map[app:1 block:11]] num_txs:0 result_begin_block:map[events:[map[attributes:[map[index:false key:aGVhZGVy value:eyJ2ZXJzaW9uIjp7ImJsb2NrIjoxMSwiYXBwIjoxfSwiY2hhaW5faWQiOiJwZWFjaHRyZWUiLCJoZWlnaHQiOjQ2ODUyLCJ0aW1lIjoiMjAyMS0wOC0yMlQxNzo0Mzo1Mi45OTU1ODY3MTRaIiwibGFzdF9ibG9ja19pZCI6eyJoYXNoIjoiUFpiN0c2VUtTYkt0aE9Rb2ZHWG40MHRYWTRMOG5vSjUwSy9WYnpsSjJzTT0iLCJwYXJ0X3NldF9oZWFkZXIiOnsidG90YWwiOjEsImhhc2giOiJhdGE4MFp6enY3ZUlFMG9zcWF6ci9JTWFOT0hNSjF3Smk2a3plcVZIanJRPSJ9fSwibGFzdF9jb21taXRfaGFzaCI6IjM4dzhRTkswRVBrMUtkRjk3VVhXbXdhRW5yR0xvUUZ4VWp5RmQ5c25yNVE9IiwiZGF0YV9oYXNoIjoiNDdERVFwajhIQlNhKy9USW1XKzVKQ2V1UWVSa201Tk1wSldaRzNoU3VGVT0iLCJ2YWxpZGF0b3JzX2hhc2giOiJaZXh0T2NEV0JUK2RrV0F6c2tFNXV1cDZVNUtzNW93ak9zL2ZoQUh0VnpBPSIsIm5leHRfdmFsaWRhdG9yc19oYXNoIjoiWmV4dE9jRFdCVCtka1dBenNrRTV1dXA2VTVLczVvd2pPcy9maEFIdFZ6QT0iLCJjb25zZW5zdXNfaGFzaCI6IkJJQ1J2SDNjS0Q5M3Y3K1IxenhFMmxqRDM0cWN2SVowQmRpMzg5cXRvaTg9IiwibGFzdF9yZXN1bHRzX2hhc2giOiI0N0RFUXBqOEhCU2ErL1RJbVcrNUpDZXVRZVJrbTVOTXBKV1pHM2hTdUZVPSIsImV2aWRlbmNlX2hhc2giOiI0N0RFUXBqOEhCU2ErL1RJbVcrNUpDZXVRZVJrbTVOTXBKV1pHM2hTdUZVPSIsInByb3Bvc2VyX2FkZHJlc3MiOiI0UERPZWplK0Z1M21mM0NESFZZSXhlcHVoVUE9In0=]] type:block]]] result_end_block:map[validator_updates:<nil>]]]\n"

@@ -333,6 +333,51 @@ func EthereumNetworkStatsDataSourceFactory(network *network.Network) *NetworkSta
}
}

type HeaderResponse struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kthomas there is a mistake in provide-go struct, this is full response, should we just go with
response.value.header

or with full response?
I will push struct you decide directly to provide-go and update version and then i think this is ready for merge

@skosito
Copy link
Contributor Author

skosito commented Aug 22, 2021

@kthomas ready for another pass
logs:

time="2021-08-22T18:42:45Z" level=debug msg="Received 3879-byte message on network stats websocket for network: peachtree baseledger"
time="2021-08-22T18:42:45Z" level=debug msg="Block header received { peachtree 048091BC7DDC283F77BFBF91D73C44DA58C3DF8A9CBC867405D8B7F3DAADA22F E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855 E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855 47521 {C731F79871284E2DD16A136336E5B939EF711A57527A9FE24901415C3AEF9D7C {2E491001C50EC36C6B6313658D8A1C647B33EEFFC1D1EA83572BE901B5109442 1}} 3528ADBDE9B79293A81C3075CC8F050A533683904BD202D7731410DEFB3A1067 E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855 65EC6D39C0D6053F9D916033B24139BAEA7A5392ACE68C233ACFDF8401ED5730 E0F0CE7A37BE16EDE67F70831D5608C5EA6E8540 2021-08-22 18:42:37.240103702 +0000 UTC 65EC6D39C0D6053F9D916033B24139BAEA7A5392ACE68C233ACFDF8401ED5730 {1 11}}\n"

@@ -0,0 +1,8 @@
#!/bin/bash
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like having separate scripts for now to not mess up with other tests, please let me know if i should somehow integrate with other scripts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants