chore(rln-relay): make account address optional #1258
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
rymnc
added
the
track:rln
rymnc
requested review from
fryorcraken,
richard-ramos,
s1fr0,
staheri14 and
jm-clius
Jenkins BuildsClick to see older builds (6)
|
s1fr0
reviewed
Oct 11, 2022
| ethClientAddr = conf.rlnRelayEthClientAddress | ||
| ethMemContractAddress = web3.fromHex(web3.Address, conf.rlnRelayEthContractAddress) | ||
| var ethAccountPrivKeyOpt = none(keys.PrivateKey) | ||
| if conf.rlnRelayEthAccountPrivateKey != "": | ||
| var ethAccountAddressOpt = none(Address) | ||
| if conf.rlnRelayEthAccountPrivateKey != "" and conf.rlnRelayCredPath != "": |
There was a problem hiding this comment.
Why do you require rlnRelayCredPath to be non-empty to populate ethereum private and public keys?
There was a problem hiding this comment.
we don't need to populate those fields if the credential path is passed in. ideally we would do this check after verifying the contents of the credentials file.
There was a problem hiding this comment.
Based on what you said it should then be something like
if conf.rlnRelayCredPath == "":
if conf.rlnRelayEthAccountPrivateKey != "":
load key
else:
raise error
There was a problem hiding this comment.
Addressed in 42d0749. If the credential path is non empty, and the credentials are valid we do not make use of the private key.
staheri14
reviewed
Oct 14, 2022
| @@ -1052,7 +1053,7 @@ proc mountRlnRelayStatic*(node: WakuNode, | |||
|
|
|||
| proc mountRlnRelayDynamic*(node: WakuNode, | |||
| ethClientAddr: string = "", | |||
| ethAccAddr: web3.Address, | |||
| ethAccountAddress: Option[web3.Address] = none(web3.Address), | |||
There was a problem hiding this comment.
When an input is optional, it is good to add the Opt suffix to its name.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Should resolve #1116 and by extension #1221
This PR removes the need to always pass in an account address. While it can be derived from the private
key, we use field temporarily. It will be removed in a subsequent PR.
Now, while restarting chat2/wakunode2 with persisted credentials, you can omit
rln-relay-eth-account-addressandrln-relay-eth-private-key.Additionally, some consistent naming has been introduced.