feat: store proof, round on block by egonspace · Pull Request #45 · Finschia/ostracon

egonspace · 2020-03-27T00:04:58Z

closes #7
closes #8

Added two field of round and proof in block.

modified createBlock adding vrf proof
modified verifyBlock verifying vrf proof

Testing

// build link using our tendermint
// go to link repository and replace tendermint module to ours
$ go mod edit -replace github.com/tendermint/tendermint=/Users/user/go/src/github.com/line/tendermint
$ make build; make install

// init blockchain and start linkd
$ sh .initialize
$ linkd start

// verifying block structure
// you can see round and proof in a block
$linkcli query block
{
  "block_id": {
    "hash": "BD2C84A3AB80C3A45539F59AE49375B38D37347572A8DE510DDBB649091C8781",
    "parts": {
      "total": "1",
      "hash": "6FE324361BBE21A863EAB91FAF2F467846515636E657EFB2A7B7DE35DC9512A7"
    }
  },
  "block": {
    "header": {
      "version": {
        "block": "10",
        "app": "0"
      },
      "chain_id": "link",
      "height": "3",
      "time": "2020-03-26T23:38:12.903301Z",
      "last_block_id": {
        "hash": "6EDBD65DF5CF48D3B6B51327B9AC583A7C493E842135B81B0390E3A0FD7116BE",
        "parts": {
          "total": "1",
          "hash": "375921117E7123F7DFA2E91336862F1A2320E0D60D205904EB3D62BA576A66FB"
        }
      },
      "last_commit_hash": "EE80912725DD3064E72D017ADE4009B20085F1DBC0D1EB1FA6AF69A86A1EC14E",
      "data_hash": "",
      "validators_hash": "AB01E18C20A4215119834DD01ED20A933EDA520984085FCBA70212067E434AAC",
      "next_validators_hash": "AB01E18C20A4215119834DD01ED20A933EDA520984085FCBA70212067E434AAC",
      "consensus_hash": "048091BC7DDC283F77BFBF91D73C44DA58C3DF8A9CBC867405D8B7F3DAADA22F",
      "app_hash": "A6D82028BE639725C2BC0AA8367FADA20681CB409A1F9DDEFB3A8675C1232284",
      "last_results_hash": "",
      "evidence_hash": "",
      "proposer_address": "D8C77E2F207738DC7043175548C24FE39436F177",
      "round": "0",
      "proof": "0312BABECD883B76714B705E5F7CDDA467218199B6F900D9B3D54CE189DB5B808D29A494CFFAAE0539BB3567A5D7D1115E074C00C9A761FE4BFE85C4D6EEF8019DBDA93F03F0AF753F71DF881E733EC955"
    },
    "data": {
      "txs": null
    },
    "evidence": {
      "evidence": null
    },
    "last_commit": {
      "height": "2",
      "round": "0",
      "block_id": {
        "hash": "6EDBD65DF5CF48D3B6B51327B9AC583A7C493E842135B81B0390E3A0FD7116BE",
        "parts": {
          "total": "1",
          "hash": "375921117E7123F7DFA2E91336862F1A2320E0D60D205904EB3D62BA576A66FB"
        }
      },
      "signatures": [
        {
          "block_id_flag": 2,
          "validator_address": "D8C77E2F207738DC7043175548C24FE39436F177",
          "timestamp": "2020-03-26T23:38:12.903301Z",
          "signature": "G2utCelTWSYfWHZ68KEOLy5b+miWTQB6Y1fAT7+BGqeZnfXUhzExfA6H0sQ6rVCjsCBh79OAB9m5P6vbKpwODA=="
        }
      ]
    }
  }
}

Referenced an issue explaining the need for the change
Updated all relevant documentation in docs
Updated all code comments where relevant
Wrote tests
Updated CHANGELOG_PENDING.md

zemyblue · 2020-03-27T03:26:27Z

state/state.go

+	b := make([]byte, 16)
+	binary.PutVarint(b, state.LastBlockHeight)
+	binary.PutVarint(b[8:], int64(round))
+	hash := tmhash.New()
+	hash.Write(seed)
+	return hash.Sum(b), nil


Is Meaning of this codes hash + height + round?
Thus, can other language like java make hash by this way?

Yes it is. hash.Write() and hash.Sum() is standard function of sha256 so other language can implement it.
tmhash is widely used in tendermint. If tmhash is not possible in other language, all other hash function has same problem.

The thmash of Tendermint appears to be SHA-256, In a 64-bit environment, SHA-512 is significantly faster. How about it? (I think we should make a better decision than following Tendermint as long as it's substantially advantageous in the scope of standard technology.)

zemyblue · 2020-03-27T03:29:02Z

state/validation.go

+		return types.NewErrInvalidProof(err.Error())
+	}
+	_, val := state.Validators.GetByAddress(block.ProposerAddress)
+	verified, err := vrf.Verify(val.PubKey.(ed25519.PubKeyEd25519), block.Proof.Bytes(), message)


If err is not nil, it does not handle.

zemyblue

LGTM

torao · 2020-03-27T12:59:30Z

types/block.go

 	ProposerAddress Address          `json:"proposer_address"` // original proposer of the block
+
+	// vrf info
+	Round int              `json:"round"`


It's better to define the type as uint32 since the int in golang varies in size depending on the compilation environment, and we don't need a sign. Related this, it's also better to be the type of the parameter using round in this PR from int to uint32.

RoundState.Round is already defined as int. I think it's better to define the type same to RoundState.Round.

torao · 2020-03-27T13:17:00Z

state/state.go

+	}
+	b := make([]byte, 16)
+	binary.PutVarint(b, state.LastBlockHeight)
+	binary.PutVarint(b[8:], int64(round))


PutVarint() is a variable-length encoding feature, and the write range may exceed 8byte depending on the output value. Since variable-length encoding isn't particularly useful here and only unnecessarily reduces portability, I think it's better to be explicitly endian and size.

binary.LittleEndian.PutUint64(b, uint64(state.LastBlockHeight)) binary.LittleEndian.PutUint64(b[8:], uint64(round))

Okay, I apply it.

torao · 2020-03-27T13:26:27Z

state/state.go

+	b := make([]byte, 16)
+	binary.PutVarint(b, state.LastBlockHeight)
+	binary.PutVarint(b[8:], int64(round))
+	hash := tmhash.New()
+	hash.Write(seed)
+	return hash.Sum(b), nil


The thmash of Tendermint appears to be SHA-256, In a 64-bit environment, SHA-512 is significantly faster. How about it? (I think we should make a better decision than following Tendermint as long as it's substantially advantageous in the scope of standard technology.)

egonspace · 2020-03-28T02:09:53Z

It's good suggestion. If we decide to use sha512, then we should just modify following code.

func New() hash.Hash {
	return sha256.New()  // -> sha512.New()
}

zemyblue · 2020-03-30T00:30:54Z

It's good suggestion. If we decide to use sha512, then we should just modify following code.
func New() hash.Hash {
	return sha256.New()  // -> sha512.New()
}

Besides this, I think there seems to be more to check.
Thus if this has many changes, how about handling in another PR?

zemyblue · 2020-03-30T01:51:12Z

@egonspace, please change the merge target to develop

egonspace · 2020-03-30T01:58:14Z

@egonspace, please change the merge target to develop

Ah, thank you. Done.

egonspace · 2020-03-30T01:59:26Z

It's good suggestion. If we decide to use sha512, then we should just modify following code.
func New() hash.Hash {
	return sha256.New()  // -> sha512.New()
}
Besides this, I think there seems to be more to check.
Thus if this has many changes, how about handling in another PR?

Sure. I think so.

torao · 2020-04-02T01:22:59Z

My previous comment was the idea that intended to make the SHA-512 only new features that we add with VRF. If we replace the Tendermint's func New() hash.Hash, I think New512_256() is better because SHA-256 and SHA-512 have different byte sizes.

egonspace · 2020-04-02T04:53:19Z

My previous comment was the idea that intended to make the SHA-512 only new features that we add with VRF. If we replace the Tendermint's func New() hash.Hash, I think New512_256() is better because SHA-256 and SHA-512 have different byte sizes.

Well, I think the reason they defined tmhash is to unify all hash functions in tendermint into one. There is no reason to use 512 in some cases and 256 in others. I think it's better to change tmhash if 512 is judged to be better in every way.

Woosang Son added 2 commits March 27, 2020 08:23

feat: store proof, round on block

f69e54e

docs: changelog_pending

03554fe

egonspace requested review from torao and zemyblue March 27, 2020 00:05

chore: print proof to hex

0e68d59

egonspace self-assigned this Mar 27, 2020

egonspace added the C: enhancement Classification: New feature or its request, or improvement in maintainability of code label Mar 27, 2020

egonspace added this to the Evolve Leader Election into VRF milestone Mar 27, 2020

chore: for pretty printing of block to hex

32f7b1c

zemyblue reviewed Mar 27, 2020

View reviewed changes

Woosang Son added 2 commits March 27, 2020 15:27

fix: race bug

06c1db2

fix: error handling

50ae22e

zemyblue approved these changes Mar 27, 2020

View reviewed changes

torao reviewed Mar 27, 2020

View reviewed changes

fix: apply comment

4f7173e

egonspace changed the base branch from master to develop March 30, 2020 01:57

torao mentioned this pull request Mar 30, 2020

Replace Tendermint's PoS to VRF-based Random Sampling #48

Closed

Woosang Son added 2 commits March 30, 2020 16:55

fix: round validation

06f124b

fix: remove redundant parameter

7fa8319

torao approved these changes Apr 2, 2020

View reviewed changes

egonspace merged commit 0168be6 into develop Apr 2, 2020

egonspace mentioned this pull request Apr 8, 2020

fix: round validation #54

Merged

5 tasks

egonspace deleted the feature/proof_in_block branch June 22, 2020 08:45

Conversation

egonspace commented Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zemyblue left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

egonspace commented Mar 28, 2020

Uh oh!

zemyblue commented Mar 30, 2020

Uh oh!

zemyblue commented Mar 30, 2020

Uh oh!

egonspace commented Mar 30, 2020

Uh oh!

egonspace commented Mar 30, 2020

Uh oh!

torao commented Apr 2, 2020

Uh oh!

egonspace commented Apr 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

egonspace commented Mar 27, 2020 •

edited

Loading