This repository has been archived by the owner on Dec 21, 2021. It is now read-only.
Validate client's Ethereum address options #223
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timoxley
approved these changes
Mar 15, 2021
| if (await mainnetProvider.getCode(this.factoryMainnetAddress) === '0x') { | ||
| throw new Error( | ||
| `Data union factory contract not found at ${this.factoryMainnetAddress}, check StreamrClient.options.dataUnion.factoryMainnetAddress!` | ||
| ) | ||
| } | ||
|
|
||
| const factoryMainnet = new Contract(this.factoryMainnetAddress!, factoryMainnetABI, mainnetWallet) | ||
| const factoryMainnet = new Contract(this.factoryMainnetAddress, factoryMainnetABI, mainnetWallet) |
| 'dataUnion.factoryMainnetAddress', | ||
| 'dataUnion.factorySidechainAddress', | ||
| 'dataUnion.templateMainnetAddress', | ||
| 'dataUnion.templateSidechainAddress' |
There was a problem hiding this comment.
Neat, as-is this is an improvement, but note this will silently stop working if a config is renamed or if new EthereumAddress config items are added. Developer always needs to remember to update this list of paths. A shame since we do already have the type information.
I wonder if we should do something like I described in slack, where we have an unsafe type at API boundaries, then a safe type internally and the only way to get from unsafe to safe type is to pass through the isAddress validator.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If user overrides some option in
StreamrClientOptions, we validate that value already in the constructor. Using the option is therefore easier in the code, as we know that the value is always valid.In this PR we validate all overriden Ethereum addresses. Open question: Should we validate also other overriden fields this way? E.g.
restUrl,maxPublishQueueSize...