Set ChainID on InitChain #1367
Set ChainID on InitChain #1367
Conversation
|
Fixes Issue #1295 |
|
Can you update the changelog? Other than that LGTM |
Codecov Report
@@ Coverage Diff @@
## develop #1367 +/- ##
===========================================
+ Coverage 63.72% 63.78% +0.06%
===========================================
Files 109 109
Lines 6026 6028 +2
===========================================
+ Hits 3840 3845 +5
+ Misses 1967 1964 -3
Partials 219 219 |
There was a problem hiding this comment.
Changes look fine, but can we now delete https://github.com/cosmos/cosmos-sdk/blob/develop/x/auth/ante.go#L78?
|
Yup, got rid of it. Could have done so a long time ago tho: https://github.com/cosmos/cosmos-sdk/blob/develop/x/auth/ante.go#L88 |
|
setCheckState was initially called in InitFromStore with an empty blockheader and reset on Commit with the current blockheader. This meant that chainID was missing from the first block's checkState, which would cause CheckTx to fail on the first block with the standard ante-handler (since signBytes included chainID) |
|
Do we know why the unit tests are failing? |
|
The latest merge broke initChainer because genesisState.StakeData is not set. Looking into fixing this now. |
|
@cwgoes bug fixed. ready for review |
baseapp/baseapp.go
Outdated
| @@ -383,6 +381,8 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg | |||
| // if this is a test and InitChain was never called. | |||
| if app.deliverState == nil { | |||
| app.setDeliverState(req.Header) | |||
| } else { | |||
| app.deliverState.ctx = app.deliverState.ctx.WithBlockHeader(req.Header) | |||
There was a problem hiding this comment.
Why is this now necessary? app.DeliverState is set to nil when Commit() is called. Also, I think the comment on line 381 is wrong for this reason.
* Added chain-id to context in InitChain * Fix bug in test * fmt * Appease linter * updated changelog * Remove chainID hack * setCheckState in InitChain * Fix bug * Fix initialization errors in example tests * Initialize app tests with default stake genesis * fix comments
* Update cosmos-hub-roadmap-2.0.md * Update cosmos-hub-roadmap-2.0.md
Closes #1295