multi: Add getblockchaininfo rpc. #1479

dnldd · 2018-10-04T16:13:47Z

This resolves #1475.

Thanks to @davecgh for his help & @matheusd for testing/review.

matheusd

Small naming issue.

I'm also seeing something weird while syncing a node in mainnet:

https://gist.github.com/matheusd/62bb07d9dfc279aa6d5bd0273f83e288

This node already had a blochain and started syncing at around block 279436.

Notice as the syncing is progressing, the "since" value of sdiff deployment is changing. Is this supposed to be happening?

matheusd · 2018-10-05T16:28:32Z

blockmanager.go

@@ -352,6 +357,13 @@ func (b *blockManager) resetHeaderState(newestHash *chainhash.Hash, newestHeight
 	}
 }

+// SyncHeight returns the current synced block height from peers.


I'd reword this slightly, because reading it, it seems that this is the block height the local node has synced, while this is meant to be the block height the local node is trying to sync to (ie: the "target" sync height). The semantics for this "syncHeight" are also slighly tricky, since it's either the current best height (on a node that has just loaded up and hasn't tried syncing to anything yet or that has lost all peers) or it's the height it's trying to sync to.

This would also be useful information returned in getblockchaininfo, since this rpc already seems to include info that is not strictly from the node's chain.

dnldd · 2018-10-06T22:09:39Z

The value for since only changes when the deployment's agenda changes state. Thanks for the review, will run some additional tests.

dnldd · 2018-10-07T23:04:57Z

@matheusd issues raised should be resolved. Here's some test output on mainnet:

{
  "chain": "mainnet",
  "blocks": 281147,
  "headers": 281147,
  "syncheight": 281147,
  "bestblockhash": "00000000000000006d29375a4c478dbd7e1f421ce1f3f8f198d6f706ecace041",
  "difficulty": 419468269,
  "verificationprogress": 1,
  "chainwork": "0x92bd67183c9f59c79d50",
  "initialblockdownload": false,
  "maxblocksize": 393216,
  "deployments": {
    "lnfeatures": {
      "version": 5,
      "status": "active",
      "since": 181504,
      "starttime": 1505260800,
      "expiretime": 1536796800
    },
    "lnsupport": {
      "version": 4,
      "status": "active",
      "since": 141184,
      "starttime": 1493164800,
      "expiretime": 1508976000
    },
    "sdiffalgorithm": {
      "version": 4,
      "status": "active",
      "since": 141184,
      "starttime": 1493164800,
      "expiretime": 1524700800
    }
  }
}

davecgh · 2018-10-08T19:42:03Z

Those since values do not appear to be correct.

DCP0001 became active at height 149248
DCP0002 became active at height 189568
DCP0003 became active at height 189568

davecgh · 2018-10-09T16:39:49Z

blockchain/thresholdstate.go

+		return 0, err
+	}
+
+	return stateChangeNode.height + 1, err


This is incorrect. The function which determines the node in question should be returning the correct node. This would return 2 in the case the genesis block node was returning which is clearly not correct!

davecgh · 2018-10-09T16:43:10Z

blockchain/thresholdstate.go

+// StateActivationHeight returns the height of the first block where the
+// current state of consensus deployment agenda activated.
+func (b *BlockChain) StateActivationHeight(hash *chainhash.Hash, version uint32, deploymentID string) (int64, error) {
+	// Fetch the block node of the provided hash.


Please include the note as described in ThresholdState as to why the node status is required to be known valid.

davecgh · 2018-10-09T16:45:14Z

blockmanager.go

@@ -334,6 +334,11 @@ type blockManager struct {
 	// The following fields are used to filter duplicate block announcements.
 	announcedBlockMtx sync.Mutex
 	announcedBlock    *chainhash.Hash
+
+	// The following fields are used to track the current synced block height


This isn't really correct. It's used to track the height that is being synced to, not the currently synced one.

davecgh · 2018-10-09T16:54:07Z

dcrjson/chainsvrresults.go

@@ -89,16 +89,29 @@ type GetBlockVerboseResult struct {
 	NextHash      string        `json:"nextblockhash,omitempty"`
 }

+// AgendaInfo provides an overview of an agenda in a consensus deployment.
+type AgendaInfo struct {
+	Version    uint32 `json:"version"`


I'm not convinced this should be included. It only has to do with voting, and the main purpose of including the deployment information in the RPC is to report the status of historical deployments.

Also, it doesn't make sense in certain circumstances, such as simnet and new versions of testnet, where changes caused by historical votes on mainnet are rolled into their respective genesis blocks and thus were never voted on for that network.

noted, updating. I initially included it for ease of use with getvoteinfo, that requires the development id and the version. getblockchaininfo would be requested first and with that the caller would have all the info needed to take a closer look at each vote since the response has the deployment id and the version.

davecgh

The documentation specifies that the new command is available to limited users (which it should be), but you didn't include the command in the map which permits its use by a limited user.

davecgh · 2018-10-09T17:44:56Z

rpcserverhelp.go

+
+	// GetBlockchainInfoResult help.
+	"getblockchaininforesult-chain":                "The current network name.",
+	"getblockchaininforesult-blocks":               "The block count.",


I'd suggest something a little more descriptive.

Maybe:
The number of blocks in the current best chain.

davecgh · 2018-10-09T17:47:31Z

rpcserverhelp.go

+	// GetBlockchainInfoResult help.
+	"getblockchaininforesult-chain":                "The current network name.",
+	"getblockchaininforesult-blocks":               "The block count.",
+	"getblockchaininforesult-headers":              "The block header count.",


Perhaps:

The number of validated block headers that comprise the target best chain.

I'm making this distinction because, while it's not currently the case, the plan is to move to a full headers-first download mode where all of the headers will be downloaded first followed by downloading all of the actual block data in parallel.

davecgh · 2018-10-09T17:52:51Z

rpcserverhelp.go

+	"getblockchaininforesult-difficulty":           "The current network difficulty.",
+	"getblockchaininforesult-verificationprogress": "The chain verification progress estimate.",
+	"getblockchaininforesult-chainwork":            "Hex encoded total work done for the chain.",
+	"getblockchaininforesult-initialblockdownload": "Syncing status",


This description doesn't seem to be helpful. I suggest:

Best guess of whether this node is in the initial block download mode used to catch up the chain when it is far behind

davecgh · 2018-10-09T17:53:21Z

rpcserverhelp.go

+	"getblockchaininforesult-initialblockdownload": "Syncing status",
+	"getblockchaininforesult-maxblocksize":         "The maximum allowed block size.",
+	"getblockchaininforesult-deployments":          "Network consensus deployments.",
+	"getblockchaininforesult-deployments--desc":    "Nonsensus deployment agendas.",


s/Nonsensus/Consensus/

davecgh

Also, the RPC server help for the AgendaInfo is missing.

davecgh · 2018-10-09T17:59:37Z

blockchain/thresholdstate.go

@@ -465,6 +465,91 @@ func (b *BlockChain) deploymentState(prevNode *blockNode, version uint32, deploy
 	return invalidState, DeploymentError(deploymentID)
 }

+// stateActivationNode returns the node at which the provided consensus
+// deployment agenda activated.


This is improperly named and the comment is incorrect. It is not when it activated, rather when the state last changed.

davecgh · 2018-10-09T18:05:12Z

blockchain/thresholdstate.go

+
+// StateActivationHeight returns the height of the first block where the
+// current state of consensus deployment agenda activated.
+func (b *BlockChain) StateActivationHeight(hash *chainhash.Hash, version uint32, deploymentID string) (int64, error) {


This is wrong too. It's not returning when it activated, rather the height at which the state last changed. For example, try calling this with the hash at height 140000.

davecgh · 2018-10-09T18:56:36Z

blockchain/thresholdstate.go

+	// the state is the same for all blocks within a given window.
+	wantHeight := calcWantHeight(svh, confirmationWindow, prevNode.height+1)
+	prevNode = prevNode.Ancestor(wantHeight)
+	firstNodeLastConfWindow := *prevNode


No need to make a copy of the node here. Just keep the pointer to it.

davecgh · 2018-10-09T19:00:45Z

blockchain/thresholdstate.go

+		}
+
+		if state.State != curState.State {
+			stateActivationNode := firstNodeLastConfWindow.Ancestor(


This won't always be correct. On the first iteration, there is no guarantee that firstNodeLastConfWindow will actually have gone back more than confirmationWindow + 1, so it would return nil in that case and end up crashing later.

davecgh · 2018-10-09T19:01:39Z

blockchain/thresholdstate.go

+			"deployment id (%s) not found", deploymentID)
+	}
+
+	aNode, err := b.stateLastChanged(version, node, checker, cache)


aNode implies activation node. Maybe stateNode?

davecgh · 2018-10-09T19:06:27Z

blockchain/thresholdstate.go

+	// Return the threshold state for the window that contains the genesis
+	// block if the chain is not past stake validation height.
+	if prevNode == nil || prevNode.height+1 < svh+confirmationWindow {
+		return b.bestChain.Genesis(), nil


Accessing bestChain requires the chain lock which is not taken. I suggest returning nil instead and having the caller check it so the lock can be avoided altogether.

davecgh · 2018-10-09T19:08:27Z

blockchain/thresholdstate.go

@@ -465,6 +465,90 @@ func (b *BlockChain) deploymentState(prevNode *blockNode, version uint32, deploy
 	return invalidState, DeploymentError(deploymentID)
 }

+// stateLastChanged returns the node at which the provided consensus
+// deployment agenda last changed state.
+func (b *BlockChain) stateLastChanged(version uint32, prevNode *blockNode, checker thresholdConditionChecker, cache *thresholdStateCache) (*blockNode, error) {


prevNode implies it is calculating something for the next node, but that is not the case here. This should just be node.

davecgh · 2018-10-09T20:21:34Z

Since we've gone back and forth several times now regarding the since heights, here is fully commented code that is tested and works properly:

// stateLastChanged returns the node at which the provided consensus deployment
// agenda last changed state.  The function will return nil if the state has
// never changed.
//
// This function MUST be called with the chain state lock held (for writes).
func (b *BlockChain) stateLastChanged(version uint32, node *blockNode, checker thresholdConditionChecker, cache *thresholdStateCache) (*blockNode, error) {
	// No state changes are possible if the chain is not yet past stake
	// validation height and had a full interval to change.
	confirmationInterval := int64(checker.RuleChangeActivationInterval())
	svh := checker.StakeValidationHeight()
	if node == nil || node.height < svh+confirmationInterval {
		return nil, nil
	}

	// Determine the current state.  Notice that thresholdState always
	// calculates the state for the block after the provided one, so use the
	// parent to get the state for the requested block.
	curState, err := b.thresholdState(version, node.parent, checker, cache)
	if err != nil {
		return nil, err
	}

	// Determine the first block of the current confirmation interval in order
	// to determine block at which the state possibly changed.  Since the state
	// can only change at an interval boudnary, loop backwards one interval at
	// a time to determine when (and if) the state changed.
	finalNodeHeight := calcWantHeight(svh, confirmationInterval, node.height)
	node = node.Ancestor(finalNodeHeight + 1)
	priorStateChangeNode := node
	for node != nil && node.parent != nil {
		// As previously mentioned, thresholdState always calculates the state
		// for the block after the provided one, so use the parent to get the
		// state of the block itself.
		state, err := b.thresholdState(version, node.parent, checker, cache)
		if err != nil {
			return nil, err
		}

		if state.State != curState.State {
			return priorStateChangeNode, nil
		}

		// Get the ancestor that is the first block of the previous confirmation
		// interval.
		priorStateChangeNode = node
		node = node.RelativeAncestor(confirmationInterval)
	}

	return nil, nil
}

// StateLastChangedHeight returns the height at which the provided consensus
// deployment agenda last changed state.  Note that, unlike the ThresholdState
// function, this function returns the information as of the passed block hash.
//
// This function is safe for concurrent access.
func (b *BlockChain) StateLastChangedHeight(hash *chainhash.Hash, version uint32, deploymentID string) (int64, error) {
	// NOTE: The requirement for the node being fully validated here is strictly
	// stronger than what is actually required.  In reality, all that is needed
	// is for the block data for the node and all of its ancestors to be
	// available, but there is not currently any tracking to be able to
	// efficiently determine that state.
	node := b.index.LookupNode(hash)
	if node == nil || !b.index.NodeStatus(node).KnownValid() {
		return 0, HashError(hash.String())
	}

	// Fetch the treshold state cache for the provided deployment id as well as
	// the condition checker.
	var cache *thresholdStateCache
	var checker thresholdConditionChecker
	for k := range b.chainParams.Deployments[version] {
		if b.chainParams.Deployments[version][k].Vote.Id == deploymentID {
			checker = deploymentChecker{
				deployment: &b.chainParams.Deployments[version][k],
				chain:      b,
			}
			cache = &b.deploymentCaches[version][k]
			break
		}
	}

	if cache == nil {
		return 0, fmt.Errorf("threshold state cache for agenda with "+
			"deployment id (%s) not found", deploymentID)
	}

	// Find the node at which the current state changed.
	b.chainLock.Lock()
	stateNode, err := b.stateLastChanged(version, node, checker, cache)
	b.chainLock.Unlock()
	if err != nil {
		return 0, err
	}

	var height int64
	if stateNode != nil {
		height = stateNode.height
	}
	return height, nil
}

In order to prove correctness of this code, I printed the information for several block heights around the interval changes and before SVH. Results:

State for block 000000000000437482b6d47f82f374cde539440ddb108b0a76886f0d87d126b9 (height 1) is ThresholdDefined -- last changed at height 0
State for block 00000000000013a36bb6fbb076214f3a33abee4eeb592d5c757c8e285d0fa5d5 (height 4095) is ThresholdDefined -- last changed at height 0
State for block 00000000000013722f8e5a8af9cf55492e9237e77d29da98695e65fd13033625 (height 4096) is ThresholdDefined -- last changed at height 0
State for block 00000000000003a57e35f399742552c4ef291c5d84fa442a408a50ddd7e827ec (height 116990) is ThresholdDefined -- last changed at height 0
State for block 00000000000003a2b61a16cb354274f742d0cba95b8994b461c35a7b66ab1218 (height 116991) is ThresholdDefined -- last changed at height 0
State for block 000000000000057f7022dc5b61c3cf1a1b550cc46ea0558bd63f0436194489c7 (height 116992) is ThresholdDefined -- last changed at height 0
State for block 0000000000000462c81f58ad946cc76fae3785af5392798bbdd171f58c4828c7 (height 116993) is ThresholdDefined -- last changed at height 0
State for block 00000000000000f43d52f48057f96618fd4f828a0cab5b50bae94e8edea2ea79 (height 125054) is ThresholdDefined -- last changed at height 0
State for block 000000000000020c5ee310650065c0b64854b4b2e213662e15c6a67be0391717 (height 125055) is ThresholdDefined -- last changed at height 0
State for block 000000000000024e882e898abda79a60305e4d3b0e9e67f8cff6191e0335c166 (height 125056) is ThresholdDefined -- last changed at height 0
State for block 00000000000000a908959c82bc460d0c925da8c7cb1281c60790ccf3b718682f (height 125057) is ThresholdDefined -- last changed at height 0
State for block 000000000000014804905b14f835e1c0fb5c1c170b8c203afffb3db3539a6d9d (height 133118) is ThresholdDefined -- last changed at height 0
State for block 00000000000001711097a83ee1f0f2c9b87ab4563b8a99df3dfc4a75082a2ccb (height 133119) is ThresholdDefined -- last changed at height 0
State for block 00000000000000796ef26549e567547f3a7fdf67cd22b478fef6f4c4a72838f6 (height 133120) is ThresholdStarted -- last changed at height 133120
State for block 00000000000001e247cd691fd2d85ee6da1a275f8347b37a52c1d48a3077a111 (height 133121) is ThresholdStarted -- last changed at height 133120
State for block 000000000000017e3d8e4ff2cd68e26aa937ad4d6b8c42ce8efe753290ad943c (height 141182) is ThresholdStarted -- last changed at height 133120
State for block 0000000000000005b8907519379dfa9d4ef8a5bd697664afe414005a2d509cfb (height 141183) is ThresholdStarted -- last changed at height 133120
State for block 00000000000000ca1eaaeebcdd21e86c95fc6f0103c8f0b8a868d4b197b46c12 (height 141184) is ThresholdLockedIn -- last changed at height 141184
State for block 00000000000001bbd1988f814fcea5deed48985bb605cfbcff9304bd0674a0ea (height 141185) is ThresholdLockedIn -- last changed at height 141184
State for block 00000000000000d680e31c405dda28d6d2114afdd32e7e986404498ff75117ba (height 149246) is ThresholdLockedIn -- last changed at height 141184
State for block 0000000000000139582d056bc20bb352f4e9b248acbb202724f46000e59c9f75 (height 149247) is ThresholdLockedIn -- last changed at height 141184
State for block 000000000000001bf88e81e7b71194e557bb4df70fda0792fd76a0d50977f789 (height 149248) is ThresholdActive -- last changed at height 149248
State for block 0000000000000073d976e8d8a1f8fb62b916b8efc3bfa96035116d4d60ae40dc (height 149249) is ThresholdActive -- last changed at height 149248
State for block 000000000000006be21825b4b39dedb1c949e66d31fac6146c95027f2f655240 (height 197630) is ThresholdActive -- last changed at height 149248
State for block 0000000000000072b9df60fcee320414357cfe9e711ff31706bf3496b5a631ba (height 197631) is ThresholdActive -- last changed at height 149248
State for block 000000000000006d1232e49378095ba723667af452ee9d0da99344c620d52702 (height 197632) is ThresholdActive -- last changed at height 149248

davecgh · 2018-10-09T21:46:55Z

rpcserver.go

+		Blocks:               best.Height,
+		Headers:              best.Height,
+		SyncHeight:           syncHeight,
+		ChainWork:            fmt.Sprintf("%#x", chainWork),


The result should not include the 0x prefix and should correctly not that work is in terms of a uint256.

Thus, it should be fmt.Sprintf("%064x", chainWork)

davecgh · 2018-10-09T21:48:37Z

blockchain/thresholdstate.go

+	node = node.Ancestor(finalNodeHeight + 1)
+	priorStateChangeNode := node
+	for node != nil && node.parent != nil {
+		// As previously mentioned, nextRhresholdState always calculates the state


s/nextRhresholdState/nextThresholdState/

davecgh · 2018-10-09T21:53:06Z

rpcserver.go

+func handleGetBlockchainInfo(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (interface{}, error) {
+	params := s.server.chainParams
+	best := s.chain.BestSnapshot()
+	dInfo := make(map[string]dcrjson.AgendaInfo)


No reason to create this so early before errors might end up making it unnecessary to create at all. Prefer defining just before first use. So just before the for loop (after the comment for the section).

Same goes for params above.

davecgh · 2018-10-09T21:54:30Z

rpcserver.go

+	}
+
+	// Guess the syncing status of the node.
+	initialBlockDownload := !s.chain.IsCurrent()


I think this should just be done directly in the response below. It's not used anywhere else.

davecgh · 2018-10-09T21:56:22Z

rpcserver.go

+	// Estimate the verification progress of the node.
+	syncHeight := s.server.blockManager.SyncHeight()
+	var verifyProgress float64
+	switch syncHeight > 0 {


Why using a switch for a single condition? Just use the zero value and do:

var verifyProgress float64 if syncHeight > 0 { verifyProgress = float64(best.Height) / float64(syncHeight) }

It's 4 lines versus 7 and more consistent with the rest of the code.

davecgh · 2018-10-09T22:02:48Z

rpcserver.go

+						"for agenda with id (%v).", agenda.Vote.Id))
+			}
+
+			if state.State != blockchain.ThresholdDefined {


No reason to get the state and avoid calling StateLastChangedHeight if the state is defined. The function will just return 0 in that case, which is the same result.

davecgh · 2018-10-09T23:12:28Z

Some results during initial sync with the latest:

{
 "chain": "mainnet",
 "blocks": 65239,
 "headers": 65239,
 "syncheight": 281735,
 "bestblockhash": "0000000000000d6f6f779e83c7c78837a1bc206ec27a0b95beeff06d8887d95d",
 "difficulty": 437488364,
 "verificationprogress": 0.23156157381936926,
 "chainwork": "000000000000000000000000000000000000000000000009337ed0b926ca8a88",
 "initialblockdownload": true,
 "maxblocksize": 393216,
 "deployments": {
   "lnfeatures": {
     "status": "defined",
     "starttime": 1505260800,
     "expiretime": 1536796800
   },
   "lnsupport": {
     "status": "defined",
     "starttime": 1493164800,
     "expiretime": 1508976000
   },
   "sdiffalgorithm": {
     "status": "defined",
     "starttime": 1493164800,
     "expiretime": 1524700800
   }
 }
}
{
  "chain": "mainnet",
  "blocks": 133190,
  "headers": 133190,
  "syncheight": 281735,
  "bestblockhash": "0000000000000127851fa41bf9d21d9cd4e6e3d8ccd961e4e503639ddf1f8032",
  "difficulty": 436334663,
  "verificationprogress": 0.472749214687561,
  "chainwork": "00000000000000000000000000000000000000000000002ae7deab0edce62a10",
  "initialblockdownload": true,
  "maxblocksize": 393216,
  "deployments": {
    "lnfeatures": {
      "status": "defined",
      "starttime": 1505260800,
      "expiretime": 1536796800
    },
    "lnsupport": {
      "status": "started",
      "since": 133120,
      "starttime": 1493164800,
      "expiretime": 1508976000
    },
    "sdiffalgorithm": {
      "status": "started",
      "since": 133120,
      "starttime": 1493164800,
      "expiretime": 1524700800
    }
  }
}
{
  "chain": "mainnet",
  "blocks": 141226,
  "headers": 141226,
  "syncheight": 281735,
  "bestblockhash": "00000000000000a4d4d2b8c62d5a605eda0f276cebbf0f24159e633575f78bae",
  "difficulty": 436343246,
  "verificationprogress": 0.5012724723587768,
  "chainwork": "000000000000000000000000000000000000000000000038adb693d55e78938e",
  "initialblockdownload": true,
  "maxblocksize": 393216,
  "deployments": {
    "lnfeatures": {
      "status": "defined",
      "starttime": 1505260800,
      "expiretime": 1536796800
    },
    "lnsupport": {
      "status": "lockedin",
      "since": 141184,
      "starttime": 1493164800,
      "expiretime": 1508976000
    },
    "sdiffalgorithm": {
      "status": "lockedin",
      "since": 141184,
      "starttime": 1493164800,
      "expiretime": 1524700800
    }
  }
}
{
  "chain": "mainnet",
  "blocks": 149344,
  "headers": 149344,
  "syncheight": 281735,
  "bestblockhash": "00000000000000258cc91b5593651d7b31afd5b2f6902cdbd0ffbb3caa65ca85",
  "difficulty": 436309838,
  "verificationprogress": 0.530086783679699,
  "chainwork": "00000000000000000000000000000000000000000000004a2e29de12fc2fb648",
  "initialblockdownload": true,
  "maxblocksize": 393216,
  "deployments": {
    "lnfeatures": {
      "status": "defined",
      "starttime": 1505260800,
      "expiretime": 1536796800
    },
    "lnsupport": {
      "status": "active",
      "since": 149248,
      "starttime": 1493164800,
      "expiretime": 1508976000
    },
    "sdiffalgorithm": {
      "status": "active",
      "since": 149248,
      "starttime": 1493164800,
      "expiretime": 1524700800
    }
  }
}

dnldd force-pushed the getblockchaininfo_rpc branch 2 times, most recently from 540343e to e52a86e Compare October 4, 2018 16:43

matheusd reviewed Oct 5, 2018

View reviewed changes

dnldd force-pushed the getblockchaininfo_rpc branch from e52a86e to 3b50153 Compare October 7, 2018 23:00

dnldd force-pushed the getblockchaininfo_rpc branch 2 times, most recently from 9baf408 to 3e42c2e Compare October 7, 2018 23:57

chappjc mentioned this pull request Oct 8, 2018

correct MaximumBlockSize is missing from /parameters decred/dcrdata#678

Closed

davecgh added this to the 1.4.0 milestone Oct 8, 2018

dnldd closed this Oct 9, 2018

dnldd reopened this Oct 9, 2018