Implement Weight method + st/gt for heaviest ts #359
Conversation
dutterbutter
requested review from
ansermino,
austinabell and
ec2
as code owners
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use address::Address; | ||
| use cid::multihash::Identity; | ||
|
|
||
| #[test] | ||
| fn genesis_test() { | ||
| let db = db::MemoryDB::default(); | ||
|
|
||
| let cs = ChainStore::new(Arc::new(db)); | ||
| let gen_block = BlockHeader::builder() | ||
| .epoch(1) | ||
| .weight((2 as u32).into()) | ||
| .messages(Cid::new_from_cbor(&[], Identity)) | ||
| .message_receipts(Cid::new_from_cbor(&[], Identity)) | ||
| .state_root(Cid::new_from_cbor(&[], Identity)) | ||
| .miner_address(Address::new_id(0).unwrap()) | ||
| .build_and_validate() | ||
| .unwrap(); | ||
|
|
||
| assert_eq!(cs.genesis().unwrap(), None); | ||
| cs.set_genesis(gen_block.clone()).unwrap(); | ||
| assert_eq!(cs.genesis().unwrap(), Some(gen_block)); |
There was a problem hiding this comment.
Can you explain why exactly this test needs to be removed? Seems like at a high level that function should be able to be called without anything else, why can this not happen?
There was a problem hiding this comment.
The test needs more setup as it fails during the weight() method called from update_heaviest(). I could follow lotus more precisely and have a put_tipset() wrapper over persist_headers() where update_heaviest() would live and have set_genesis() only call persist_headers() since it only takes in a blockheader anyway. Otherwise, I will refactor the test to account for new required setup.
There was a problem hiding this comment.
what is the setup that's missing though? that genesis block not existing in the blockstore? seems intuitive that this should not require other setup, but can look more into specifics later
There was a problem hiding this comment.
No, the setup that's missing is STORAGE_POWER_ACTOR_ADDR not being available in the weight method. I am adding a put_tipset method, similar to lotus; see here. As it seems we should not be calling update_heaviest() upon initializing the node with genesis.
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use address::Address; | ||
| use cid::multihash::Identity; | ||
|
|
||
| #[test] | ||
| fn genesis_test() { | ||
| let db = db::MemoryDB::default(); | ||
|
|
||
| let cs = ChainStore::new(Arc::new(db)); | ||
| let gen_block = BlockHeader::builder() | ||
| .epoch(1) | ||
| .weight((2 as u32).into()) | ||
| .messages(Cid::new_from_cbor(&[], Identity)) | ||
| .message_receipts(Cid::new_from_cbor(&[], Identity)) | ||
| .state_root(Cid::new_from_cbor(&[], Identity)) | ||
| .miner_address(Address::new_id(0).unwrap()) | ||
| .build_and_validate() | ||
| .unwrap(); | ||
|
|
||
| assert_eq!(cs.genesis().unwrap(), None); | ||
| cs.set_genesis(gen_block.clone()).unwrap(); | ||
| assert_eq!(cs.genesis().unwrap(), Some(gen_block)); |
There was a problem hiding this comment.
what is the setup that's missing though? that genesis block not existing in the blockstore? seems intuitive that this should not require other setup, but can look more into specifics later
Summary of changes
Changes introduced in this pull request:
Weightmethodload_heaviest_tipsetto be used when starting the nodeset_heaviest_tipsetStoragePowerto BigUintReference issue to close (if applicable)
Closes #118 #327
Other information and links