Conversation
| Delay uint32 `default:"20" usage:"how many states should pass before snapshot is produced"` | ||
| LocalPath string `default:"waspdb/snap" usage:"the path to the snapshots folder in this node's disk"` | ||
| NetworkPaths []string `default:"" usage:"the list of paths to the remote (http(s)) snapshot locations; each of listed locations must contain 'INDEX' file with list of snapshot files"` | ||
| SnapshotToLoad string `default:"" usage:"block hash of a snapshot to load"` |
There was a problem hiding this comment.
Notice config.json structure has changed in this place. We would need to update all configs upon deployment.
| if storedChainRec.ChainID() != chainRecord.ChainID() { | ||
| return fmt.Errorf("cannot update chain record with different chain ID: %v != %v", | ||
| storedChainRec.ChainID().String(), chainRecord.ChainID().String()) | ||
| } |
There was a problem hiding this comment.
Added this check to ensure chain ID cannot change. This is simillar to what we had - Delete registry method is not exposed and Set had chain ID as it's argument. Now we still do not expose Delete, but we not dont have chain ID argument, so need to ensure nothing gets overwritten.
| var params = &app.ComponentParams{ | ||
| Params: map[string]any{ | ||
| "chains": ParamsChains, | ||
| "chain": ParamsChainRunner, |
There was a problem hiding this comment.
Notice this key also changed in config.json.
| }() | ||
| defer e.Shutdown(context.Background()) | ||
|
|
||
| time.Sleep(5 * time.Second) |
There was a problem hiding this comment.
Should fix sporadic issues in this test
There was a problem hiding this comment.
I guess the system back online this part, but should be good to add it but smaller number too
There was a problem hiding this comment.
Yeah, I just moved this line, but sure it makes sense to also decrease the time. Will do
|
|
||
| // Protocol implementation. | ||
| type cmtLogImpl struct { | ||
| chainID isc.ChainID // Chain, for which this log is maintained by this committee. |
There was a problem hiding this comment.
Notice chain ID was removed from this and many other messages in chain/chains folder. @kape1395 could you please review this part?
And in general, could removing chain ID from consensus code cause issues? E.g. make chain vulnerable to some attacks like replay attack
| func (c *ChainRunner) initSnapshotToLoad(config string) { | ||
| c.snapshotToLoad = nil | ||
| blockHash, err := state.BlockHashFromString(config) | ||
| if err != nil { | ||
| c.log.LogErrorf("Parsing snapshots to load: %s is not a block hash: %v", config, err) | ||
| return |
There was a problem hiding this comment.
Now there is only one line, with syntax as it was for default snapshot
packages/registry/chain_registry.go
Outdated
| } | ||
|
|
||
| type ChainRecordRegistryImpl struct { | ||
| chainID isc.ChainID // We only have one chain. Multi-chain code left for backwards compatibility. |
There was a problem hiding this comment.
Now registry supports only one chain record. But to avoid need to update all the files we keep implementation mostly as is, with exception that interface of the registry was updated to not require chain ID for some methods. And to implement it, we store chain ID of that one chain (inside map onChange callback).
| hasAccess bool | ||
| isServer bool |
There was a problem hiding this comment.
Are these good names?
There was a problem hiding this comment.
This and other files in this folder were not created but moved. But IDK for some reason this is not diplayed even though I used git mv
| func getConfig(chainID int) *params.ChainConfig { | ||
| if c, ok := configCache.Get(chainID); ok { | ||
| if c := configCache.Load(); c != nil { |
There was a problem hiding this comment.
I think it's necessary to check the value of chainID here. Because some solo tests may still be multi-chain.
4 participants
Changes worth to note:
snapshotsToLoadarray became single stringsnapshotToLoadwhich is just block hash (without chain ID). Now only one snapshot can be loadedchainskey becamechain