Changed proto definition along with converting system. #24
Conversation
| string NotarizedHeaderHash = 1; | ||
| ShardOutportBlock OutportBlock = 2 ; | ||
| string NotarizedHeaderHash = 1; | ||
| ShardOutportBlock OutportBlock = 2; |
There was a problem hiding this comment.
i think we need to reference ShardOutportBlockV2 here
There was a problem hiding this comment.
chenged back. let's keep the structure untouched for now, and actually remove versioning for the shardblocks altogether in a separate PR. What do you think?
| @@ -13,3 +13,14 @@ func (x *ShardOutportBlock) GetHeaderNonce() (uint64, error) { | |||
|
|
|||
| return x.BlockData.Header.Nonce, nil | |||
| } | |||
|
|
|||
| func (y *ShardOutportBlockV2) GetHeaderNonce() (uint64, error) { | |||
There was a problem hiding this comment.
i think we can remove the structs that will not be used anymore
| if err != nil { | ||
| panic(err) | ||
| } |
There was a problem hiding this comment.
same to all other places where panic is used
There was a problem hiding this comment.
yes, sorry for that. leftovers when I was testing ad-hoc.
process/outportBlockConverter.go
Outdated
| IntraShardMiniBlocks: intraShardMiniBlocks, | ||
| } | ||
|
|
||
| var err error |
There was a problem hiding this comment.
we don't need to declare this error variable here
| } | ||
|
|
||
| func (o *outportBlockConverter) handleBlockData(blockData *outport.BlockData, shardOutportBlock *hyperOutportBlocks.ShardOutportBlockV2) error { | ||
| if blockData == nil { |
There was a problem hiding this comment.
is there a case where blockData can be nil? if yes, shouldn't we return an error if that's happending, instead of returning nil?
There was a problem hiding this comment.
I don't remember why I put that check. But I'll switch to error.
| @@ -184,6 +760,24 @@ func (o *outportBlockConverter) castBigInt(i *big.Int) ([]byte, error) { | |||
| return buf, err | |||
| } | |||
|
|
|||
| func copyMiniBlocks(sourceMiniBlocks []*block.MiniBlock) []*hyperOutportBlocks.MiniBlock { | |||
| var destMiniBlocks []*hyperOutportBlocks.MiniBlock | |||
| if sourceMiniBlocks != nil { | |||
There was a problem hiding this comment.
we don't need this nil check, ranging over sourceMiniBlocks will "treat" the nil case
There was a problem hiding this comment.
right, but if they are not nil but empty, the destMiniBlock will be nil instead of empty.
There was a problem hiding this comment.
yes, but if you remove also L764, i think it will be fine; or maybe you can init like make([]*hyperOutportBlocks.MiniBlock, len(sourceMiniBlocks)
There was a problem hiding this comment.
if I init, I won't have parity between the two, since the initialized will never be nil, even though the source is nil.
process/outportBlockConverter.go
Outdated
| shardBlockData.Header = &hyperOutportBlocks.Header{} | ||
| header := shardBlockData.Header |
There was a problem hiding this comment.
we can construct header variable at first, and the set to shardBlockData.Header, no need to set empty header first and then reference the header with a new variable
There was a problem hiding this comment.
optional suggestion: set simple fields directly when creating new header variable &hyperOutportBlocks.Header{} and set slices later on
process/outportBlockConverter.go
Outdated
| destTxInfo := &hyperOutportBlocks.TxInfoV2{} | ||
|
|
||
| // TxInfo - Transaction | ||
| if txInfo.Transaction != nil { |
There was a problem hiding this comment.
revert if condition to avoid indentation? if transaction == nil --> continue
| @@ -40,6 +42,347 @@ func NewOutportBlockConverter( | |||
| }, nil | |||
| } | |||
|
|
|||
| func (o *outportBlockConverter) HandleShardOutportBlockV2(outportBlock *outport.OutportBlock) (*hyperOutportBlocks.ShardOutportBlockV2, error) { | |||
There was a problem hiding this comment.
comments for exported methods
No description provided.