Conversation
b7ea146 to
526d19a
Compare
| if block.InnerBlock != nil { | ||
| return errZeroBlockHasInnerBlock | ||
| } | ||
| if prevBlock.InnerBlock.Digest() != sm.LastNonSimplexInnerBlock.Digest() { |
There was a problem hiding this comment.
how was this passing before? prevBlock.InnerBlock is supposed to be nil after the if
There was a problem hiding this comment.
I think because prevBlock.InnerBlock is sm.LastNonSimplexInnerBlock
| Seq: 43, | ||
| Epoch: 43, | ||
| Prev: preSimplexParent.Digest(), | ||
| Prev: preSimplexParent.InnerBlock.Digest(), |
There was a problem hiding this comment.
but we are calling StateMachine.Digest which encodes the inner blocks digest. So with this change this says the last non-simplex block is no longer wrapped in simplex protocol metadata?
There was a problem hiding this comment.
we're building on top of the pre-simplex parent, so its digest is computed with the inner block.
The outer block doesn't exist for the pre-simplex parent, it's just a facade because we encode the ICMEpochInfo in the test outside of it.
| parsedBlock := &ParsedBlock{ | ||
| msm: e.msm, | ||
| StateMachineBlock: block, | ||
| legacyBlock: seq <= e.lastNonSimplexHeight, |
There was a problem hiding this comment.
wouldn't it be easier to just wrap any non-simplex blocks in the default protocol metadata? this way we wouldn't need the legacy block fields and wouldn't need to use inner.Digest
There was a problem hiding this comment.
the protocol metadata doesn't contain the hash though, and the hash is computed differently for pre-simplex blocks.
Previously to this commit, when building the first ever simplex block, simplex was using the previous block hash of the protocol metadata, which is computed differently and incorrectly because the block previous to the first ever simplex block is not a simplex block. Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
eb4cdfb to
5970701
Compare
| return errZeroBlockInnerDigestMismatch | ||
|
|
||
| // The zero block must build upon the last non-Simplex block | ||
| if block.Metadata.SimplexProtocolMetadata.Prev != sm.LastNonSimplexInnerBlock.Digest() { |
There was a problem hiding this comment.
do we also verify the sequence is as expected?
There was a problem hiding this comment.
We do it via line 974, because the expected block's digest depends on the sequence.
We need this because we don't have a better way of ensuring the previous block points to the non-simplex block.
I can add an explicit check.
| type CachedStorage struct { | ||
| msm *metadata.StateMachine | ||
| lock sync.RWMutex | ||
| lastNonSimplexHeight uint64 |
| return nil, nil, err | ||
| } | ||
|
|
||
| legacy := seq <= cs.lastNonSimplexHeight |
There was a problem hiding this comment.
when are we going to need to retrieve something from storage < lastNonSimplexHeight? Aren't we only going to need to retrieve the lastNonSimplex block?
e75238c to
67d3125
Compare
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
328f077 to
bddd3e3
Compare
2 participants
Previously to this commit, when building the first ever simplex block, simplex was using the previous block hash of the protocol metadata, which is computed differently and incorrectly because the block previous to the first ever simplex block is not a simplex block.