Skip to content

cli: show timestamp of the block#3

Open
RichardWeiYang wants to merge 24 commits intoJeiwan:masterfrom
RichardWeiYang:master
Open

cli: show timestamp of the block#3
RichardWeiYang wants to merge 24 commits intoJeiwan:masterfrom
RichardWeiYang:master

Conversation

@RichardWeiYang
Copy link
Copy Markdown

Display the timestamp of block in printchain command.

Merkle tree requires a full binary tree, which means the number of node is
power of 2 and the level of the tree is log(N) instead of N/2.

This patch duplicate the node to a power of 2 number and correct the level
calculation. Besides it adds a test case with 6 nodes.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
The leading "zeroBytes" in address is b58Alphabet[0] instead of 0x00.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
We could only send money from our own wallet.

If the parameter is not an address in our wallet, exit the program gracefully
and show an error message.
@Jeiwan
Copy link
Copy Markdown
Owner

Jeiwan commented Oct 27, 2017

Wow! Thanks for your commits! I'll review them on Saturday.

Copy link
Copy Markdown
Owner

@Jeiwan Jeiwan left a comment

Choose a reason for hiding this comment

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

Awesome! But I cannot make it a package, because it should be a standalone app. Also, the exploring commands are just for debugging/testing, so they shouldn't be a part of the codebase. And the HTML part shouldn't be in the main package as well. It can be a part of a web-server package, and there could be a 'web-server' CLI command that starts the server. And the server would use the blockchain and read data from it. Eventually, this could be a web-based interfaces for the blockchain, where one can see things happening on the node, as well as send/receive coins.
If you're going to make a web-interface, could you please submit it in a separate PR, so I could merge the important improvements and bug fixes of this PR? Thanks!

Comment thread base58.go
@@ -1,4 +1,4 @@
package main
package bc
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm afraid this is against the idea of the project: it should be a standalone app, not a package. Why did you decide to make this change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sure, this is up to you. I just add it for my purpose.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

hmm, actually, I am not comfortable with the GitHub pull request.

Since what I want you to take is the fix on Decoder and merkle tree, the #2 - #4 commit.

While GitHub automatically create the pull request with all my new commits on master branch.

How could I send a pull request just with dedicated commit? or you could pick up the one you want? or I have to make a clean master branch so that you can pull?

Comment thread block.go

func (b *Block) PrintHTML(detail bool) string {
var lines []string
lines = append(lines, fmt.Sprintf("<h2>Block <a href=\"/block/%x\">%x</a> </h2>", b.Hash, b.Hash))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It's better to do this via 'html/template', and this should be a separate package, because it's not directly related to the logic of the blockchain.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agree, I just learn go, so...

Comment thread cli_explore.go
@@ -0,0 +1,48 @@
package bc
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Not sure about this file and commands. Looks like its purpose is just to debug/test things.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yep, this is some cli commands which helps me to understand the blockchain:-)

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.

3 participants