[R4R] publish block info for thirdpart partner #667

unclezoro · 2019-09-17T06:41:44Z

Description

resolve #668
Third part requires some extra info and more common info(just like other chain) to audit.

So we add a new topic to publish more block info.

related pr bnb-chain/bnc-cosmos-sdk#167

Rationale

Here is the schema

blockdata.json.zip

Notice that the metrics changed: PublishBlockTimeMs is not more the total time, rename to PublishTotalTimeMs.

Notice: Account have an new field: sequence , all downstream service need update the schema @cpzhao @erhenglu @fletcher142

Example

{
    "ChainID":"bnbchain-1000",
    "CryptoBlock":{
        "BlockHash":"febcb38e6f73a6e8a9a25edd34af1a1243d262cf552fe5b6d8e8d5e56151db47",
        "ParentHash":"0f09bcca609ecbf5c5ad448f7cc1c0e8f4cb7d350383dfac21f5854e2960304b",
        "BlockHeight":615,
        "Timestamp":"2019-09-17T06:36:47.998467Z",
        "TxTotal":29,
        "BlockMeta":{
            "LastCommitHash":"b08199aee98218bcf6b2df027df324d2cf1a00c8d34d64831266edb5f7ea972d",
            "DataHash":"28ae673938db1acc616e0549682648f47e60419ebb87d7f707c98f36cc66ee94",
            "ValidatorsHash":"df37cb39c9cfd4bdd65853e79e329938ecb5c968f2bf2cee643cabab0c8c073d",
            "NextValidatorsHash":"df37cb39c9cfd4bdd65853e79e329938ecb5c968f2bf2cee643cabab0c8c073d",
            "ConsensusHash":"294d8fbd0b94b767a7eba9840f299a3586da7fe6b5dead3b7eecba193c400f93",
            "AppHash":"28fe85c8ee8f0f89b152f55837182bb6f4f1fe89ac276712b509a6d9ef335545",
            "LastResultsHash":"",
            "EvidenceHash":"",
            "ProposerAddress":"bca132gd9ucywlg5n0fpp7grjfcwc3pymfpw3y0ncc"
        },
        "Transactions":[
            {
                "TxHash":"9FF02C6EA789C1CC77E87CA32D570C658C70D2BCC3FC9F088BE979650D7BD520",
                "Fee":"37500BNB",
                "Timestamp":"2019-09-17T06:36:47.998467Z",
                "Inputs":[
                    {
                        "Address":"bnb1hxg5aml7wgvzspqhdxx6xvl8z9hwaw8ndn6nwa",
                        "Coins":null
                    }
                ],
                "Outputs":null,
                "BnbTransaction":{
                    "Source":0,
                    "TxType":"depositHTLT",
                    "TxAsset":"",
                    "OrderId":"",
                    "Code":0,
                    "Data":"{"from":"bnb1hxg5aml7wgvzspqhdxx6xvl8z9hwaw8ndn6nwa","amount":[{"denom":"ETH-806","amount":10000}],"swap_id":"a67ed223cea923710bbcf9f171548b3d378342c5a5b129831a537bb23d1adb51"}"
                }
            }
        ]
    }
}

Changes

Notable changes:

add each change in a bullet point here
...

Preflight checks

build passed (make build)
tests passed (make test)
Race test-case in app/app_test package is failed. Even in master it fails, @xiangdotli @rickyyangz know about this. won't fix in thsi pr
integration tests passed (make integration_test)
manual transaction test passed (cli invoke)

Already reviewed by

...

Related issues

resolve #668

unclezoro · 2019-09-18T06:51:01Z

Gopkg.lock

@@ -499,6 +507,14 @@
  revision = "9a47f48565a795472d43519dd49aac781f3034fb"
  version = "v1.6.0"

+[[projects]]


you can ignore deadlock and goid depnendency, I will reorg Gopkg.lock and Gopkg.toml before merge.

unclezoro · 2019-09-18T07:10:06Z

common/tx/pool.go

@@ -0,0 +1,37 @@
+package tx


There is also a pool.go file in cosmos. Why I choose put here: 1. cosmos-sdk won't use this file. 2. the pool.go in cosmos use sync.map, which is not suit for this scene

the name pool is a little weird, tx.pool looks like a tx mempool, How about name it TxResults and put it under app/pub package

I move it into app/pub package, but still name it as pool.go, for now it only store txResult, but it can store other things in the future.

ackratos · 2019-09-18T12:35:46Z

app/config/config.go

+publishBlock = {{ .PublicationConfig.PublishBlock }}
+blockTopic = "{{ .PublicationConfig.BlockTopic }}"
+blockKafka = "{{ .PublicationConfig.BlockKafka }}"
+
 # Global setting
 publicationChannelSize = "{{ .PublicationConfig.PublicationChannelSize }}"


can you help me remove the double-quote? :) Its my mistake

ackratos · 2019-10-02T10:21:02Z

Gopkg.toml

@@ -42,7 +42,7 @@
 [[constraint]]
  name = "github.com/cosmos/cosmos-sdk"
  source = "github.com/binance-chain/bnc-cosmos-sdk"
-  version = "=v0.25.0-binance.19"
+  branch = "blockhash"


Will change right?

sure, after cosmos release or merged

you can change back to develop branch

changed back to develop

app/pub/helpers.go

ackratos · 2019-10-02T10:54:42Z

app/pub/msgs.go

@@ -44,11 +47,12 @@ func (this msgType) String() string {
 // figure out which version of writer schema to use.
 // This allows consumers be deployed independently (in advance) with publisher
 var latestSchemaVersions = map[msgType]int{
-	accountsTpe:        0,
+	accountsTpe:        1,


Please notice me and kaiqiang when this is going to deploy

app/pub/schema_test.go

app/pub/schemas.go

app/pub/helpers.go

ackratos · 2019-10-03T02:14:39Z

app/pub/helpers.go

+			Logger.Error("tx contains no messages", "hash", txhash)
+			return true
+		}
+		// Notice: if support multi message in one transaction in future, this part need change


We’d better Make a todo comment with issue number.

ackratos · 2019-10-03T02:22:56Z

app/pub/helpers.go

+		inputs := []Input{{Address: msg.GetSigners()[0].String()}}
+
+		switch msg := msg.(type) {
+		case orderPkg.NewOrderMsg:


I suggest if time allowed we add interface to each message which is more maintainable

I think there is no need for the abstraction. Cipher trace's requirement is special, no need to change the basic interface of Msg for them. txAsset, inputs, outputs is more like bitcoin concept, most message do not have such interface.

ackratos · 2019-10-14T10:11:16Z

app/pub/publisher_kafka.go

@@ -32,6 +32,7 @@ type KafkaMarketDataPublisher struct {
 	transfersCodec        *goavro.Codec
 	blockCodec            *goavro.Codec

+	failFast         bool


for which nodes we enable this?

we won't enable this, but cypher trace will.

app/apptest/match_allocation_fill_new_test.go

app/apptest/match_allocation_split_new_test.go

abelliumnt · 2019-10-25T07:24:02Z

app/app.go

@@ -463,15 +463,18 @@ func (app *BinanceChain) CheckTx(txBytes []byte) (res abci.ResponseCheckTx) {
 // Implements ABCI
 func (app *BinanceChain) DeliverTx(txBytes []byte) (res abci.ResponseDeliverTx) {
 	res = app.BaseApp.DeliverTx(txBytes)
+	txHash := cmn.HexBytes(tmhash.Sum(txBytes)).String()


txHash have been calculated for serveral times in deliverTx. We should pass the txHash to inner functions

This is really an issue that bothers us long, since it is not the purpose of this pr, let us do it in . #683

rickyyangz · 2019-10-25T08:59:54Z

app/pub/msgs.go

+	Timestamp   string
+	TxTotal     int64
+
+	BnbBlockMeta BlockMeta


do not use bnb, use NativeToken instead

It is a requirement from cipher trace: They will handle multi chain message, they want to us to distinguish common block message and bnb specified message. As you see, CryptoBlock is actually totall metadata of block, while BnbBlockMeta only hold the metadata only binance chain have.

rickyyangz · 2019-10-25T09:00:21Z

app/pub/msgs.go

+	Inputs  []Input
+	Outputs []Output
+
+	BnbTransaction BnbTransaction


what's BnbTransaction mean

The message only transaction of Binance chain have.

rickyyangz · 2019-10-25T09:03:18Z

app/config/config.go

+# whether the kafka open SASL_PLAINTEXT auth
+auth = {{ .PublicationConfig.Auth }}
+kafkaUserName = "{{ .PublicationConfig.KafkaUserName }}"
+kafkaPassword = "{{ .PublicationConfig.KafkaPassword }}"


is it secure to put username and password in the config file

We won't use this. Actually there are some offline method can solve this, decipher the password and start the node, then encrypt the password. If going to support encryption of password, have to store decryption material in some hardware or cloud service, more requirement for the hardware. Just leave it to cipher trace.

gofmt & goimports update dependent add auth to cipher publish fail fast fix review suggestion add change log fix import change back to develop branch

app/fee_distribution.go

app/pub/helpers.go

unclezoro changed the title ~~[WIP]add cipher~~ [WIP]publish block info for third part Sep 17, 2019

unclezoro force-pushed the cipher branch 4 times, most recently from a8dda8b to 25f907a Compare September 17, 2019 09:55

unclezoro changed the title ~~[WIP]publish block info for third part~~ [WIP]publish block info for thirdpart partner Sep 17, 2019

unclezoro force-pushed the cipher branch 2 times, most recently from 4a83bbb to 4f84c3c Compare September 18, 2019 06:47

unclezoro commented Sep 18, 2019

View reviewed changes

unclezoro force-pushed the cipher branch from 4f84c3c to 7f9f1a3 Compare September 18, 2019 07:06

unclezoro changed the title ~~[WIP]publish block info for thirdpart partner~~ [R4R] publish block info for thirdpart partner Sep 18, 2019

unclezoro requested review from ackratos, abelliumnt, rickyyangz and yutianwu and removed request for ackratos September 18, 2019 07:07

unclezoro commented Sep 18, 2019

View reviewed changes

unclezoro force-pushed the cipher branch 2 times, most recently from 1dbbe6c to 8504592 Compare September 19, 2019 03:02

ackratos requested changes Oct 3, 2019

View reviewed changes

ackratos reviewed Oct 14, 2019

View reviewed changes

unclezoro mentioned this pull request Oct 23, 2019

[R4R] fix account may have currency with zero balance #679

Merged

4 tasks

yutianwu reviewed Oct 23, 2019

View reviewed changes

app/apptest/match_allocation_fill_new_test.go Outdated Show resolved Hide resolved

yutianwu reviewed Oct 23, 2019

View reviewed changes

app/apptest/match_allocation_split_new_test.go Outdated Show resolved Hide resolved

unclezoro changed the base branch from master to develop October 23, 2019 09:51

unclezoro force-pushed the cipher branch from d7e3f85 to db769e3 Compare October 23, 2019 10:05

ackratos approved these changes Oct 25, 2019

View reviewed changes

abelliumnt reviewed Oct 25, 2019

View reviewed changes

rickyyangz reviewed Oct 25, 2019

View reviewed changes

unclezoro force-pushed the cipher branch 3 times, most recently from 5177b28 to aaee617 Compare October 28, 2019 11:17

add cipher

3f2d626

gofmt & goimports update dependent add auth to cipher publish fail fast fix review suggestion add change log fix import change back to develop branch

unclezoro force-pushed the cipher branch from aaee617 to 3f2d626 Compare October 28, 2019 11:22

move pool.go under app/pub package

7f7f0cf

yutianwu approved these changes Oct 29, 2019

View reviewed changes

abelliumnt reviewed Oct 29, 2019

View reviewed changes

app/fee_distribution.go Show resolved Hide resolved

abelliumnt reviewed Oct 29, 2019

View reviewed changes

app/pub/helpers.go Show resolved Hide resolved

abelliumnt approved these changes Oct 30, 2019

View reviewed changes

abelliumnt merged commit 59a431f into develop Oct 30, 2019

unclezoro deleted the cipher branch May 10, 2022 06:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[R4R] publish block info for thirdpart partner #667

[R4R] publish block info for thirdpart partner #667

unclezoro commented Sep 17, 2019 •

edited

Loading

unclezoro Sep 18, 2019

unclezoro Sep 18, 2019

rickyyangz Oct 28, 2019

unclezoro Oct 29, 2019 •

edited

Loading

ackratos Sep 18, 2019

unclezoro Oct 23, 2019

ackratos Oct 2, 2019

unclezoro Oct 23, 2019

rickyyangz Oct 25, 2019

unclezoro Oct 28, 2019

ackratos Oct 2, 2019

unclezoro Oct 23, 2019

ackratos Oct 3, 2019

ackratos Oct 3, 2019

unclezoro Oct 23, 2019

ackratos Oct 14, 2019

unclezoro Oct 23, 2019

abelliumnt Oct 25, 2019

unclezoro Oct 28, 2019

rickyyangz Oct 25, 2019

unclezoro Oct 28, 2019 •

edited

Loading

rickyyangz Oct 25, 2019

rickyyangz Oct 25, 2019

unclezoro Oct 28, 2019

rickyyangz Oct 25, 2019

unclezoro Oct 28, 2019 •

edited

Loading

[R4R] publish block info for thirdpart partner #667

[R4R] publish block info for thirdpart partner #667

Conversation

unclezoro commented Sep 17, 2019 • edited Loading

Description

Rationale

Example

Changes

Preflight checks

Already reviewed by

Related issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unclezoro Oct 29, 2019 • 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

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

unclezoro Oct 28, 2019 • 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

unclezoro Oct 28, 2019 • edited Loading

Choose a reason for hiding this comment

unclezoro commented Sep 17, 2019 •

edited

Loading

unclezoro Oct 29, 2019 •

edited

Loading

unclezoro Oct 28, 2019 •

edited

Loading

unclezoro Oct 28, 2019 •

edited

Loading