fix: evm chain param validation #1264
Conversation
milapsheth
requested review from
cgorenflo,
haiyizxx,
jcs47 and
fish-sammy
as code owners
| return nil | ||
| } | ||
| } | ||
|
|
||
| if err := validateBytes(m.GatewayCode); err != nil { |
There was a problem hiding this comment.
Validate bytecode first
There was a problem hiding this comment.
Why? what bug does this fix?
There was a problem hiding this comment.
On devnet, I used the old bytecode params and it doesn't include absorber. So add-chain succeeded, but VoteConfirmChainRequest fails when setting the param set. add-chain should also validate the bytecode properly.
There was a problem hiding this comment.
The bug is that if the network is found in the networks list, we return nil right away instead of validating the bytecode.
There was a problem hiding this comment.
ok then the fix should be that if the network isn't in the list, return an error, otherwise continue instead of return. Simply reordering would leave the problem there when someone wants to add more validation.
There was a problem hiding this comment.
Sure, I prefer that anyways. Still getting used to the preferred coding style here lol
| return nil | ||
| } | ||
| } | ||
|
|
||
| if err := validateBytes(m.GatewayCode); err != nil { |
There was a problem hiding this comment.
ok then the fix should be that if the network isn't in the list, return an error, otherwise continue instead of return. Simply reordering would leave the problem there when someone wants to add more validation.
Description
Todos
Steps to Test
Expected Behaviour
Other Notes