Update application registry when chains change. #42
Conversation
nodiesBlade
changed the title
…lsllc/os-gateway into bug/41/applications-stake-change
| } | ||
|
|
||
| // Gateway operator may have updated chains staked | ||
| if !strings.EqualFold(strings.Join(networkApp1.Chains, ","), strings.Join(networkApp2.Chains, ",")) { |
There was a problem hiding this comment.
@nodiesBlade what if the order of Chain IDs is different for two applications?
Let's say:
- networkApp1.Chains = ["111", "222", "333"]
- networkApp2.Chains = ["222", "333", "111"]
I don't know if Chains are ordered before assigning to PoktApplication, but it is better not to rely on string concatenation equality.
There was a problem hiding this comment.
+ worth adding a unit test for such a case.
There was a problem hiding this comment.
Due to the nature of blockchain and how hashing works, if the chains ordering change, i would argue the application is still technically different and we should recache it. and so that's why I took a little shortcut.
Though I'm glad you mentioned it as well, wouldn't mind adding the extra security. will push up some changes for it
There was a problem hiding this comment.
Yeah, I was talking only about the way Chains were compared.
Absolutely agree with what you say about Applications equivalency on different Chains order. No need to re-cache such.
Github issue
#41
Description
This addresses #41 by detecting if applications change by adjusting the equal function to check for chains field as well.
Type of change
Please delete option that is not relevant.
Related PRs
List related PRs below