StateManager documentation #393
Conversation
Initial `StateManager` documentation. In the process of writing this a couple of inconsistencies in the code were found. Firstly several methods, which should only be called when there are no outstanding checkpoints, were found and updated to error when this is not the case. Secondly the `checkpoint` function was made asynchronous to match other methods.
| - `opts.origin` **[Buffer][44]** the address where the call originated from. The address should be a `Buffer` of 20bits. Defaults to `0` | ||
| - `opts.value` **[Buffer][44]** the value in ether that is being sent to `opt.address`. Defaults to `0` | ||
| - `cb` **[runCode~callback][45]** callback | ||
| - `cb` **[runTx~callback][41]** the callback |
There was a problem hiding this comment.
Had a look at the generated doc, looks good.
|
|
||
| [68]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Error | ||
|
|
||
| [69]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean |
There was a problem hiding this comment.
Same hear, looks good, nice and clear descriptions.
| self.stateManager.revert(function () { | ||
| cb(err) | ||
| }) | ||
| } else { |
| proto.generateCanonicalGenesis = function (cb) { | ||
| var self = this | ||
|
|
||
| if (self._checkpointCount !== 0) { return cb(new Error('Cannot create genesis state with uncommitted checkpoints')) } | ||
|
|
||
| this.hasGenesisState(function (err, genesis) { | ||
| if (!genesis && !err) { | ||
| self.generateGenesis(genesisStates[self._common.chainName()], cb) |
There was a problem hiding this comment.
Here there is a call from a method from the interface to a method from the default implementation, this is probably not intended?
There was a problem hiding this comment.
If I understand you right it's completely intended. The method generateCanonicalGenesis in the implementation can use whatever mechanism it likes to provide implementation. This is much like how putContractStorage uses _modifyContractStorage internally.
FYI, I did consider removing generateCanonicalGenesis from the interface completely, but it would require further changes as to how we initialize the state trie. Depending on feedback during the beta this is something we might change still.
There was a problem hiding this comment.
Ah, getting closer with my understanding. So if we extract/separate StateManager to an own repository, we would also extract the default StateManger, is that correct? First thought it would be rather intended to only extract the interface methods.
There was a problem hiding this comment.
We would extract both to a single repo yes. Presuming it would be Typescript, we would then define the interface along with a reference (default) implementation in the external repo with both types being exported. Now there is a question as to whether the reference implementation should include methods beyond the interface? My gut instinct is it should not and we should strip away the extra functionality. It just felt like too much to do as part of this refactor
There was a problem hiding this comment.
Yes, I would go very much along with you, also think there shouldn't be additional methods in the default implementation. Ok, but nevertheless, let's keep it for now, I might make an additional comment in the release notes or so.
Will now prepare release notes, probably do a release one day later (absolutely no problem, just for info), so likely tomorrow.
Initial
StateManagerdocumentation as per #391.In the process of writing this a couple of inconsistencies in the code were found. Firstly several methods, which should only be called when there are no outstanding checkpoints, were found and updated to error when this is not the case. Secondly the
checkpointfunction was made asynchronous to match other methods.