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

make creating genesis block independent of addr and sig #157

Merged
merged 4 commits into from
Oct 7, 2018

Conversation

MrAngelll
Copy link
Contributor

No description provided.

@MrAngelll MrAngelll requested a review from a team as a code owner October 6, 2018 00:22
@codecov
Copy link

codecov bot commented Oct 6, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@671e2b7). Click here to learn what that means.
The diff coverage is 79.59%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #157   +/-   ##
=========================================
  Coverage          ?   69.09%           
=========================================
  Files             ?       82           
  Lines             ?     8960           
  Branches          ?        0           
=========================================
  Hits              ?     6191           
  Misses            ?     2021           
  Partials          ?      748
Impacted Files Coverage Δ
blockchain/genesis.go 77.64% <79.59%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 671e2b7...17600ad. Read the comment docs.

Copy link
Contributor

@zjshen14 zjshen14 left a comment

Choose a reason for hiding this comment

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

Awesome! This will save us a lot of time on keeping updating the signatures and addresses.

Some minor comments

Recipient string `yaml:"recipient"`
Signature string `yaml:"signature"`
Amount int64 `yaml:"amount"`
RecipientPub string `yaml:"recipientPub"`
Copy link
Contributor

Choose a reason for hiding this comment

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

RecipientPub -> RecipientPK?

}

// DecodeKey decodes the string keypair
func decodeKey(pubK string, priK string) (pk keypair.PublicKey, sk keypair.PrivateKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually we put private methods (with lowerCamelCase) after public method

return
}

// GenerateAddr returns the string address according to public key
Copy link
Contributor

Choose a reason for hiding this comment

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

method name doesn't match the comment

CreatorPrivKey: "d2df3528ff384d41cc9688c354cd301a09f91d95582eb8034a6eff140e7539cb17b53401",
}

// DecodeKey decodes the string keypair
Copy link
Contributor

Choose a reason for hiding this comment

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

Method name doesn't match the comment (case)

pk, sk := decodeKey(nominator.PubKey, nominator.PriKey)
address := generateAddr(cfg.Chain.ID, pk)
vote, err := action.NewVote(
0,
Copy link
Contributor

Choose a reason for hiding this comment

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

@lizhefeng, would you please comment if 0 nonce is correct or not?

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 correct. Meant to be 0.

tsf, err := action.NewTransfer(
0, big.NewInt(transfer.Amount), Gen.CreatorAddr(cfg.Chain.ID), transfer.Recipient, []byte{}, 0, big.NewInt(0))
0,
Copy link
Contributor

Choose a reason for hiding this comment

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

@lizhefeng same here

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 correct. Meant to be 0.

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 have refactored the code

@zjshen14 zjshen14 merged commit 9e2bcc6 into iotexproject:master Oct 7, 2018
@zjshen14
Copy link
Contributor

zjshen14 commented Oct 7, 2018

@MrAngelll congrats on the first PR. Looking forward to more contributions

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.

None yet

3 participants