En 6315/dns at genesis #1981
En 6315/dns at genesis #1981
Conversation
genesis/interface.go
Outdated
| InitialSmartContracts() []InitialSmartContractHandler | ||
| IsInterfaceNil() bool | ||
| } | ||
|
|
||
| // TxExecutionProcessor represents a transaction builder and executor containing also related helper functions | ||
| type TxExecutionProcessor interface { | ||
| ExecuteTransaction(nonce uint64, sndAddr []byte, rcvAddress []byte, value *big.Int, data []byte) error | ||
| AccountExists(address []byte) bool | ||
| AccountExists(address []byte) (state.UserAccountHandler, bool) |
There was a problem hiding this comment.
Rename to GetAccount() perhaps?
There was a problem hiding this comment.
would leave like this.
There was a problem hiding this comment.
GetAcount is more appropriate. it gets the account now
| return err | ||
| } | ||
|
|
||
| account, ok := dp.AccountExists(scAddress) |
There was a problem hiding this comment.
Not sure why not checking this before ExecuteTransaction instead.
There was a problem hiding this comment.
copy paste verification remained.
There was a problem hiding this comment.
no need for this now
There was a problem hiding this comment.
it is checked before. check is needed here as well as you do not want the node to panic. although that means super big coding problem.
|
|
||
| // IsInterfaceNil returns if underlying object is true | ||
| func (dp *deployLibrarySC) IsInterfaceNil() bool { | ||
| return dp == nil || dp.TxExecutionProcessor == nil |
There was a problem hiding this comment.
TxExecutionProcessor shouldn't be nil if we have a non-nil (correctly constructed) dp. Right?
There was a problem hiding this comment.
@andreibancioiu we always need to check this since it is an exported element by construction.
genesis/interface.go
Outdated
| InitialSmartContracts() []InitialSmartContractHandler | ||
| IsInterfaceNil() bool | ||
| } | ||
|
|
||
| // TxExecutionProcessor represents a transaction builder and executor containing also related helper functions | ||
| type TxExecutionProcessor interface { | ||
| ExecuteTransaction(nonce uint64, sndAddr []byte, rcvAddress []byte, value *big.Int, data []byte) error | ||
| AccountExists(address []byte) bool | ||
| AccountExists(address []byte) (state.UserAccountHandler, bool) |
There was a problem hiding this comment.
GetAcount is more appropriate. it gets the account now
| @@ -144,14 +154,40 @@ func (scp *smartContractParser) InitialSmartContractsSplitOnOwnersShards( | |||
|
|
|||
| var smartContracts = make(map[uint32][]genesis.InitialSmartContractHandler) | |||
| for _, isc := range scp.initialSmartContracts { | |||
| shardID := shardCoordinator.ComputeId(isc.OwnerBytes()) | |||
| if isc.Type == genesis.DNSType { | |||
There was a problem hiding this comment.
I know we talk about this but now the parser has an exception: it knows we have a special contract that is of type DNS. :(
There was a problem hiding this comment.
yep. it does not need to split in that case. it also has type delegation.
| func GenerateInitialPublicKeys( | ||
| baseAddress []byte, | ||
| shardCoordinator sharding.Coordinator, | ||
| allShards bool, |
There was a problem hiding this comment.
This is ugly. Why not having 2 separate functions for each usecase?
There was a problem hiding this comment.
2 functions doing the same thing, calling a basic stuff. receiving a function pointer if ok / not ok ?
There was a problem hiding this comment.
almost the same thing, bool parameters that alter the execution of a function is not a good pattern. Indeed, code duplication should be avoided
| return err | ||
| } | ||
|
|
||
| account, ok := dp.AccountExists(scAddress) |
There was a problem hiding this comment.
no need for this now
|
|
||
| // IsInterfaceNil returns if underlying object is true | ||
| func (dp *deployLibrarySC) IsInterfaceNil() bool { | ||
| return dp == nil || dp.TxExecutionProcessor == nil |
There was a problem hiding this comment.
@andreibancioiu we always need to check this since it is an exported element by construction.
| return err | ||
| } | ||
|
|
||
| isForCurrentShard := func([]byte) bool { |
There was a problem hiding this comment.
Perhaps add an inline comment to explain this, for future reference?
# Conflicts: # core/constants.go # genesis/process/intermediate/standardDelegationProcessor.go
| @@ -95,8 +93,12 @@ func (dp *deployLibrarySC) Deploy(sc genesis.InitialSmartContractHandler) error | |||
| return err | |||
| } | |||
|
|
|||
| isForCurrentShard := func(address []byte) bool { | |||
There was a problem hiding this comment.
this could have been a non-anonymous function, also, might have signaled this as a handler: isForCurrentShardFn
deployment and computation of sc addresses for the DNS contracts at genesis time