Added new way to configure preactivation of features at any height. #1307
Conversation
Integration tests moved to use a new way of preactivation with height.
alexeykiselev
requested review from
nickeskov and
irina-pereiaslavskaia
and removed request for
nickeskov
pkg/state/state.go
Outdated
| return err | ||
| } | ||
| activationRequest := &activatedFeaturesRecord{activationHeight: fah.Height} | ||
| if err := s.stor.features.activateFeature(fah.ID, activationRequest, blockID); err != nil { |
There was a problem hiding this comment.
This method adds an activation record. But it's incorrect, because newestIsActivated will return true even if the blockchain height is lower than activation height.
The same can be applied for the approval stage and newestIsApproved method.
I think the good decision is to include activation hooks inside stateManager.blockchainHeightAction.
There was a problem hiding this comment.
Nope, it's totally correct. The idea of preactivation is to activate some features at genesis block prior everything else. I agree that activation of feature at genesis block but at certain height looks weird, but it was made for testing purposes only and must be used only for this. "Our" way of preactivation of features in genesis block and at genesis block height is the most correct way of preactivation and this way must be used for custom blockchains. That's why I save it for backward compatibility.
There was a problem hiding this comment.
I simplified the configuration structure. Removed the unnecessary new structure and made it looks like in scala implementation.
Integration tests moved to use a new way of preactivation with height.