fix: upgrade non-determinism #10188
Comments
|
Hey @tomtau, any chance you could provide a bit more info here? This issue really isn't saying much. |
|
@alexanderbez sure, reposting the original quick info from Discord:
|
|
So where does the non-determinism come from in the state machine? There isn't anything we can achieve with this issue atm. If there is any non-determinism in the SDK, the simulations would've caught it. |
|
@alexanderbez do any simulations have cross-SDK version upgrades (0.42->0.44)? One potential source of non-determinism may be in upgrade migrations: #10189 (comment) |
|
Simulations don't simulate/test cross-version live upgrades -- they simulate the state machine only. If there is non-determinism across upgrades, then that sounds like an issue in your upgrade handler? |
|
@alexanderbez the upgrade handler doesn't have anything special besides the Cosmos SDK's 0.42->0.44 migrations: https://github.com/crypto-org-chain/chain-main/blob/v3.1.1/app/app.go#L464 |
|
Yes @AmauryM explained it to me -- iteration over modules was non-deterministic because we were using a map. |
… value
This pass exists to curtail non-determinism in the cosmos-sdk
which stemmed from iterating over maps during upgrades and that
caused a chaotic debug for weeks. With this change, we'll now
enforce and report failed iterations, with the rule being that
a map in a range should involve ONLY one of these 2 operations:
* for k := range m { delete(m, k) } for fast map clearing
* for k := range m { keys = append(keys, k) } to retrieve keys & sort
thus we shall get this report:
```shell
[gosec] 2021/11/09 03:18:57 Rule error: *sdk.mapRanging => the value in the range statement should be nil: want: for key := range m (main.go:19)
[gosec] 2021/11/09 03:18:57 Rule error: *sdk.mapRanging => the value in the range statement should be nil: want: for key := range m (main.go:27)
```
from the code below:
```go
package main
func main() {
m := map[string]int{
"a": 0,
"b": 1,
"c": 2,
"d": 3,
}
makeMap := func() map[string]string { return nil }
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
values := make([]int, 0, len(m))
for _, value := range m {
values = append(values, value)
}
type kv struct {
k, v interface{}
}
kvL := make([]*kv, 0, len(m))
for k, v := range m {
kvL = append(kvL, &kv{k, v})
}
for k := range m {
delete(m, k)
}
for k := range makeMap() {
delete(m, k)
}
for k := range do() {
delete(m, k)
}
}
func do() map[string]string { return nil }
```
Updates cosmos/cosmos-sdk#10189
Updates cosmos/cosmos-sdk#10188
Updates cosmos/cosmos-sdk#10190
… value
This pass exists to curtail non-determinism in the cosmos-sdk
which stemmed from iterating over maps during upgrades and that
caused a chaotic debug for weeks. With this change, we'll now
enforce and report failed iterations, with the rule being that
a map in a range should involve ONLY one of these 2 operations:
* for k := range m { delete(m, k) } for fast map clearing
* for k := range m { keys = append(keys, k) } to retrieve keys & sort
thus we shall get this report:
```shell
[gosec] 2021/11/09 03:18:57 Rule error: *sdk.mapRanging => the value in the range statement should be nil: want: for key := range m (main.go:19)
[gosec] 2021/11/09 03:18:57 Rule error: *sdk.mapRanging => the value in the range statement should be nil: want: for key := range m (main.go:27)
```
from the code below:
```go
package main
func main() {
m := map[string]int{
"a": 0,
"b": 1,
"c": 2,
"d": 3,
}
makeMap := func() map[string]string { return nil }
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
values := make([]int, 0, len(m))
for _, value := range m {
values = append(values, value)
}
type kv struct {
k, v interface{}
}
kvL := make([]*kv, 0, len(m))
for k, v := range m {
kvL = append(kvL, &kv{k, v})
}
for k := range m {
delete(m, k)
}
for k := range makeMap() {
delete(m, k)
}
for k := range do() {
delete(m, k)
}
}
func do() map[string]string { return nil }
```
Updates cosmos/cosmos-sdk#10189
Updates cosmos/cosmos-sdk#10188
Updates cosmos/cosmos-sdk#10190
Summary of Bug
post-upgrade block may have a different result app hash.
Version
0.44.0
Steps to Reproduce
I did not find a way to reliably reproduce it so far :(
For Admin Use
The text was updated successfully, but these errors were encountered: