correctly initialize state factory #1647
Conversation
blockchain/blockchain.go
Outdated
| if bc.tipHeight == 0 { | ||
| return bc.sf.Initialize(bc.config, bc.registry) | ||
| if sfHeight == 0 { | ||
| if err := bc.sf.Initialize(bc.config, bc.registry); err != nil { |
There was a problem hiding this comment.
shadow: declaration of "err" shadows declaration at line 305 (from govet)
Codecov Report
@@ Coverage Diff @@
## master #1647 +/- ##
==========================================
+ Coverage 54.22% 54.26% +0.04%
==========================================
Files 155 155
Lines 13480 13504 +24
==========================================
+ Hits 7309 7328 +19
Misses 5226 5226
- Partials 945 950 +5
Continue to review full report at Codecov.
|
|
If we want to handle that corner case, I think it would be better to move the sf.initialize() into sf.start() so that it can initialize by itself if needed |
fixed in latest push |
| @@ -29,46 +28,33 @@ import ( | |||
| ) | |||
|
|
|||
| // createGenesisStates initialize the genesis states | |||
| func createGenesisStates(cfg config.Config, registry *protocol.Registry, ws WorkingSet) error { | |||
| func createGenesisStates(ctx context.Context, cfg config.Config, ws WorkingSet) error { | |||
There was a problem hiding this comment.
hugeParam: cfg is heavy (1488 bytes); consider passing it by pointer (from gocritic)
| return err | ||
| } | ||
| _ = ws.UpdateBlockLevelInfo(0) | ||
| return nil | ||
| } | ||
|
|
||
| func createAccountGenesisStates(ctx context.Context, sm protocol.StateManager, registry *protocol.Registry, cfg config.Config) error { | ||
| p, ok := registry.Find(account.ProtocolID) | ||
| func createAccountGenesisStates(ctx context.Context, sm protocol.StateManager, cfg config.Config) error { |
There was a problem hiding this comment.
hugeParam: cfg is heavy (1488 bytes); consider passing it by pointer (from gocritic)
| @@ -80,8 +66,9 @@ func createAccountGenesisStates(ctx context.Context, sm protocol.StateManager, r | |||
| return ap.Initialize(ctx, sm, addrs, balances) | |||
| } | |||
|
|
|||
| func createRewardingGenesisStates(ctx context.Context, sm protocol.StateManager, registry *protocol.Registry, cfg config.Config) error { | |||
| p, ok := registry.Find(rewarding.ProtocolID) | |||
| func createRewardingGenesisStates(ctx context.Context, sm protocol.StateManager, cfg config.Config) error { | |||
There was a problem hiding this comment.
hugeParam: cfg is heavy (1488 bytes); consider passing it by pointer (from gocritic)
|
|
||
| "github.com/ethereum/go-ethereum/common/hexutil" | ||
| "github.com/iotexproject/go-pkgs/hash" | ||
| "github.com/iotexproject/iotex-address/address" | ||
| "go.uber.org/zap" | ||
|
|
There was a problem hiding this comment.
File is not goimports-ed with -local github.com/iotexproject/iotex-core (from goimports)
| "github.com/iotexproject/iotex-proto/golang/iotextypes" | |
| "github.com/pkg/errors" | |
state/factory/factory.go
Outdated
| @@ -160,6 +158,17 @@ func (sf *factory) Start(ctx context.Context) error { | |||
| if err := sf.dao.Start(ctx); err != nil { | |||
| return err | |||
| } | |||
| // check factory height | |||
| _, err := sf.dao.Get(AccountKVNameSpace, []byte(CurrentHeightKey)) | |||
| if err != nil && errors.Cause(err) == db.ErrNotExist { | |||
There was a problem hiding this comment.
switch errors.Cause(err) {
case db.ErrNotExist:
...
case nil:
break
default:
return err
}
state/factory/factory.go
Outdated
| case nil: | ||
| break | ||
| case db.ErrNotExist: | ||
| if err := sf.dao.Put(AccountKVNameSpace, []byte(CurrentHeightKey), byteutil.Uint64ToBytes(0)); err != nil { |
There was a problem hiding this comment.
shadow: declaration of "err" shadows declaration at line 162 (from govet)
state/factory/factory.go
Outdated
| return errors.Wrap(err, "failed to init factory's height") | ||
| } | ||
| // init the state factory | ||
| if err := sf.initialize(ctx); err != nil { |
There was a problem hiding this comment.
shadow: declaration of "err" shadows declaration at line 162 (from govet)
state/factory/statedb.go
Outdated
| case nil: | ||
| break | ||
| case db.ErrNotExist: | ||
| if err := sdb.dao.Put(AccountKVNameSpace, []byte(CurrentHeightKey), byteutil.Uint64ToBytes(0)); err != nil { |
There was a problem hiding this comment.
shadow: declaration of "err" shadows declaration at line 109 (from govet)
state/factory/statedb.go
Outdated
| return errors.Wrap(err, "failed to init statedb's height") | ||
| } | ||
| // init the state factory | ||
| if err := sdb.initialize(ctx); err != nil { |
There was a problem hiding this comment.
shadow: declaration of "err" shadows declaration at line 109 (from govet)
| return errors.Wrap(err, "failed to init factory's height") | ||
| } | ||
| // init the state factory | ||
| if err = sf.initialize(ctx); err != nil { |
There was a problem hiding this comment.
sloppyReassign: re-assignment to err can be replaced with err := sf.initialize(ctx) (from gocritic)
| return errors.Wrap(err, "failed to init statedb's height") | ||
| } | ||
| // init the state factory | ||
| if err = sdb.initialize(ctx); err != nil { |
There was a problem hiding this comment.
sloppyReassign: re-assignment to err can be replaced with err := sdb.initialize(ctx) (from gocritic)
it could happen that bc.tipHeight > 0 while state factory DB file has been deleted
sf.Initialize() should be called based upon sf.Height() == 0, but not bc.tipHeight == 0
Test plan:
make test && make minicluster