Throw error in case of a bad merge - #384
Conversation
| nextRegister.latestFetchedBlock.blockNumber => merged | ||
| | DynamicContractRegister(_) => | ||
| | DynamicContractRegister({nextRegister}) => | ||
| if register.latestFetchedBlock.blockNumber > nextRegister.latestFetchedBlock.blockNumber { |
There was a problem hiding this comment.
I want to validate whether this case is happening. It should never but I have know way of validating without adding a log. I could just add a log and stop it from crashing but it would be invalid if we ever hit this.
Registers should only merge when the latestFetchedBlockNumber is equal.
1e9ea98 to
0bc3eaa
Compare
DZakh
left a comment
DZakh
left a comment
There was a problem hiding this comment.
Looks good, but I'd prefer to avoid unreachable code to decrease maintenance burden.
| let exists = dcr.contractAddress == contractAddress | ||
| if exists { | ||
| warnIfAttemptedAddressRegisterOnDifferentContracts( | ||
| ~contractAddress, | ||
| ~contractName, | ||
| ~existingContractName=(dcr.contractType :> string), | ||
| ~chainId, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Will it ever reach this code with an exception thrown from addAddress?
There was a problem hiding this comment.
Yes it can because the error thrown will only occur for an individual contractAddressMapping when the address actually gets added or if two mappings are combined etc.
There is no global contractAddressMapping.
What was happening before is checkContractIsRegistered would return false in the case that the address is exists but the contractName does not match. Now it returns true so it stops it from overwriting but there is no feedback to the user that this is happening and no way for us to trace it without a log. We don't need the log but it can be helpful for debugging.
2 participants
No description provided.