Explorer V2 #202

oluwandabira · 2017-09-26T07:47:18Z

Explorer V2

For issue #198 and #138

Current issues being addressed in V2:

Explore page blocks always show 0 transactions
Not showing Coinbase on Coinbase transaction page
Vote transaction page not showing Prevout Address and shows "Pending" in Output block column
SIGUSR1 doesn't reload the templates (probably has to do with the directory structure)
Properly document the package and remove excess comments

Improvements over V1:

Mining fee shown on block page (not sure if it's correct though)
Improved code organization
Completely separate explorer from the api
Use websockets to show current block height on all explorer pages and update some info on each page (mostly just the confirmations)
Tests

oluwandabira · 2017-10-03T21:19:59Z

This is ready for review and merge. The only issue is the upstream vote vin issue, all unchecked items will be added in a later pr.

chappjc

Not tested, but this really cleans things up. Will test.

chappjc · 2017-10-03T21:46:14Z

dcrsqlite/apisource.go

+
+	getTotalSpent := func(txs []*explorer.TxBasic) (total float64) {
+		for _, tx := range txs {
+			total += tx.Fee.ToCoin()


"spent" is the sum of the input values. What's going on here with tx.Fee?

I assumed total sent is the sum of the input values and total spent is the sum of the fees

The miner fees for a transaction are sum(inputs) - sum(outputs). Perhaps it's confusing, but I refer to the sum of inputs as spent since it's how much you as the sender spend to make the tx happen, while sent is how much you are sending to the output address(es). The difference is the fees that the miner gets.

The fees for a block could be just that sum total += tx.Fee.ToCoin() over all transactions tx. Or it could be done the way you started it by computing sum of sum of inputs, but then it would need sum of sum of outputs for the difference to get mining fee.

Double check against the Insight explorer to be sure it makes sense.

Changed MiningFee calculation to sum fees in over all tx except votes. I checked and the tx fees are the same on the Insight explorer.

chappjc · 2017-10-03T21:47:50Z

dcrsqlite/apisource.go

+
+	block.TotalSent = getTotalSent(block.Tx) + getTotalSent(block.Revs) + getTotalSent(block.Tickets)
+	block.TotalSpent = getTotalSpent(block.Tx) + getTotalSpent(block.Revs) + getTotalSpent(block.Tickets)
+	block.MiningFee = block.TotalSent - block.TotalSpent


for a valid tx, spent >= sent
(inputs - outputs = fee)

chappjc · 2017-10-03T21:49:44Z

dcrsqlite/apisource.go

+		log.Errorf("GetRawTransactionVerbose failed for: %v", txhash)
+		return nil
+	}
+	msgTx := txhelpers.MsgTxFromHex(txraw.Hex)


check for nil

chappjc · 2017-10-03T21:53:30Z

dcrsqlite/apisource.go

+
+	inputs := make([]explorer.Vin, 0, len(txraw.Vin))
+	for i, vin := range txraw.Vin {
+		coinbase := vin.Coinbase


please make a TODO that when Stakebase becomes a usable vin, we need to handle that too.
I'm OK with reusing explorer.Vin.CoinBase for both stakebase and coinbase, but we need to get the right one from vin depending on the type of tx

chappjc · 2017-10-03T21:56:45Z

explorer/explorertypes.go

+type Vin struct {
+	TxID            string
+	CoinBase        string
+	Address         string


Assuming this is the previous outpoint's address(es)?

A vout, and thus this vin's prevout, may have multiple addresses decoded from their pkScript.

chappjc · 2017-10-03T22:06:38Z

explorer/explorer.go

@@ -1,4 +1,7 @@
-package main
+// Package explorer handles the explorer subsystem for displaying the explorer pages
+// Copyright (c) 2017, The Dcrdata developers


👍 Let's update the LICENSE file too.
Although, I'm leaning toward all lowercase as our universal formatting, despite being a proper noun:
The dcrdata.org developers or The dcrdata developers

chappjc · 2017-10-03T22:19:03Z

explorer/explorer.go

 		r.Route("/{blockhash}", func(rd chi.Router) {
-			rd.Use(exp.app.BlockHashPathOrIndexCtx)
+			rd.Use(exp.blockHashPathOrIndexCtx)


It looked like the blockPage is only using getBlockHashCtx now, so can't this middleware be a simpler handler that does not try to get the height when it's not needed? Or or is this route intended to handle height's input here instead of a hash?

It handles both hash and height so http://localhost:7777/explorer/block/174288 and http://localhost:7777/explorer/block/000000000000000d1653f66e60c269e7fcbace5841005624ae47d680561d1b14 link to the same page

chappjc · 2017-10-03T22:20:33Z

explorer/explorermiddleware.go

@@ -0,0 +1,124 @@
+package explorer


Prepend license notification stuff

chappjc · 2017-10-03T22:21:36Z

explorer/explorermiddleware.go

+
+func (exp *explorerUI) blockHashPathOrIndexCtx(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// hash := chi.URLParam(r, "blockhash")


you can nuke all these comments

chappjc · 2017-10-03T22:27:58Z

views/tx.tmpl

-                    <th class="text-center">DCR</th>
+		<thead>
+                   <th>Address</th>
+		   <th class="text-center">Type</th>


minor formatting inconsistency

chappjc · 2017-10-03T22:29:09Z

Needs rebase.

chappjc · 2017-10-04T04:40:53Z

dcrsqlite/apisource.go

+func makeExplorerTxBasic(data dcrjson.TxRawResult, msgTx *wire.MsgTx) *explorer.TxBasic {
+	tx := new(explorer.TxBasic)
+	tx.TxID = data.Txid
+	tx.FormattedSize = humanize.Bytes(uint64(len(data.Hex)))


size is double. need data.Hex/2

chappjc · 2017-10-04T04:54:20Z

Cool. I'm also seeing an vote version column header but no vote version. Perhaps because it just needs a rebase still.

oluwandabira · 2017-10-04T05:11:05Z

Not a rebase, a rework. The vote version stuff was dependent on the explorer V1 system. If @McEdward could take a look and add it in, that'll be great.

chappjc · 2017-10-04T05:50:32Z

Well that's unfortunate. Should not really be your problem that we just put the voteinfo PR in first. Sorry about that.

@McEdward please provide guidance or better yet a commit on top of this branch that @RogueElement can cherry pick

chappjc · 2017-10-04T07:14:35Z

aaaand I just made this infinitely harder on you. sorry. The restyling just went in. With a decent 3-way merge tool this shouldn't be horrible though.

chappjc · 2017-10-04T07:27:51Z

@gozart1 Some pretty hefty conflicts landed here following your redesign PR, which was inevitable, but if there are any non-trivial or non-obvious edits that seem to conflict with dabira's, he could probably use a hand sorting it out.

Thanks and sorry for the complications. Several PRs had to go in at almost the same time.

gozart1 · 2017-10-04T16:15:39Z

I can take a look at this in about 12hrs from now.

The deltas a huge because the layout change a lot. But the actual content that #202 modifies is not that large. So I suggest the following strategy:

Copy and paste these templates

views/address.tmpl
views/block.tmpl
views/explorer.tmpl
views/tx.tmpl

directly from master, then by looking over your existing 13 commits, manually re-apply the changes you made to the tmpl files in this PR to them. One way to easily see all the changes would be to diff this branch with the commit in master before my PR was merged.

You may want to create a 2nd branch to be extra safe and just keep this one around for reference. Once you've copied the templates and applied the changes the diff should be clean and we can double check to make sure all your changes made it in.

Fixed explore and block pages Added tx page Added Address page Fixed Tx and Address pages Fixed minor errors and started ro remove explorer v1 Removed unnecessary conversion Removed redundant function Change IsStakeBase check for now Added non-redundant function Updated LICENSE and added minor changes

oluwandabira · 2017-10-04T21:36:15Z

Squashed and rebased, just needs a review and merge

chappjc · 2017-10-04T22:00:28Z

dcrsqlite/apisource.go

+		return nil
+	}
+	msgTx := txhelpers.MsgTxFromHex(txraw.Hex)
+	if err != nil {


oops, you're checking err here instead of msgTx

chappjc · 2017-10-04T22:03:59Z

views/tx.tmpl

-                                <a href="/explorer/address/{{.}}">{{.}}</a>
+                        <td>
+                            {{range .Addresses}}
+                                <a class="break-word mono" href="/explorer/address/{{.}}">{{.}}</a><br>


Just want to make sure losing the class="break-word mono" part was intentional since the rebase was pretty epic. a break-word also disappeared way above too so I'm guessing this wasn't a mistake, but I want to be sure.

* Started on Explorer Version 2 Fixed explore and block pages Added tx page Added Address page Fixed Tx and Address pages Fixed minor errors and started ro remove explorer v1 Removed unnecessary conversion Removed redundant function Change IsStakeBase check for now Added non-redundant function Updated LICENSE and added minor changes * Fixed MiningFee calculation and removed unused function * Corrected msgTx error check

gozart1 mentioned this pull request Sep 27, 2017

layout rework wip #203

Merged

oluwandabira changed the title ~~[WIP] Explorer V2~~ Explorer V2 Oct 3, 2017

chappjc requested changes Oct 3, 2017

View reviewed changes

oluwandabira force-pushed the ExplorerV2 branch from 8ef4b9f to 2fe6cbd Compare October 4, 2017 03:52

chappjc approved these changes Oct 4, 2017

View reviewed changes

chappjc reviewed Oct 4, 2017

View reviewed changes

oluwandabira added 2 commits October 4, 2017 23:33

Fixed MiningFee calculation and removed unused function

2835f87

oluwandabira force-pushed the ExplorerV2 branch from 03667f5 to 2835f87 Compare October 4, 2017 21:34

chappjc reviewed Oct 4, 2017

View reviewed changes

Corrected msgTx error check

e592253

chappjc merged commit 28e42b3 into decred:master Oct 4, 2017

chappjc mentioned this pull request Oct 5, 2017

Show transaction fees #138

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explorer V2 #202

Explorer V2 #202

oluwandabira commented Sep 26, 2017 •

edited

Loading

oluwandabira commented Oct 3, 2017

chappjc left a comment

chappjc Oct 3, 2017

oluwandabira Oct 4, 2017

chappjc Oct 4, 2017

oluwandabira Oct 4, 2017

chappjc Oct 3, 2017

chappjc Oct 3, 2017

chappjc Oct 3, 2017

chappjc Oct 3, 2017 •

edited

Loading

chappjc Oct 3, 2017

chappjc Oct 3, 2017

oluwandabira Oct 4, 2017

chappjc Oct 4, 2017

chappjc Oct 3, 2017

chappjc Oct 3, 2017

chappjc Oct 3, 2017

chappjc commented Oct 3, 2017

chappjc Oct 4, 2017

chappjc commented Oct 4, 2017

oluwandabira commented Oct 4, 2017

chappjc commented Oct 4, 2017

chappjc commented Oct 4, 2017

chappjc commented Oct 4, 2017 •

edited

Loading

gozart1 commented Oct 4, 2017

oluwandabira commented Oct 4, 2017

chappjc Oct 4, 2017

chappjc Oct 4, 2017

Explorer V2 #202

Explorer V2 #202

Conversation

oluwandabira commented Sep 26, 2017 • edited Loading

Explorer V2

oluwandabira commented Oct 3, 2017

chappjc left a 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

chappjc Oct 3, 2017 • 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

chappjc commented Oct 3, 2017

Choose a reason for hiding this comment

chappjc commented Oct 4, 2017

oluwandabira commented Oct 4, 2017

chappjc commented Oct 4, 2017

chappjc commented Oct 4, 2017

chappjc commented Oct 4, 2017 • edited Loading

gozart1 commented Oct 4, 2017

oluwandabira commented Oct 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oluwandabira commented Sep 26, 2017 •

edited

Loading

chappjc Oct 3, 2017 •

edited

Loading

chappjc commented Oct 4, 2017 •

edited

Loading