-
Notifications
You must be signed in to change notification settings - Fork 1
Add remote signer support to controller templates #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add conditional logic to set REMOTE_SIGNER_ENABLED and PRIV_VALIDATOR_LADDR when ValidatorKey is not supplied, for both CCV and non-CCV templates. Also expose port 26658 for the privval server when using remote signer.
emanuelconunaemme
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The part to test it with the current controller should be changed. We might also require a version check, but we can skip it if we are able to upgrade all the chains to v0.15 which we should.
| value: "%%% stake_owner_address %%%" | ||
| - name: KEYPASSWD | ||
| value: "%%% keychain_password %%%" | ||
| {{- if .ValidatorKey }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct version would be:
{% if remote_signer_enabled %}
- name: REMOTE_SIGNER_ENABLED
value: "true"
- name: PRIV_VALIDATOR_LADDR
value: "tcp://0.0.0.0:26658"
{% endif %}
No key is passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean controller_remote_signer_enabled ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to expand - ValidatorKey is only provided by the new controller (controller-ccv role). In the old version (controller role) there is no such variable and deployment will fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rkollar ! I was actually relaying on the lack of ValidatorKey. I just added value: "{{ .ValidatorKey }}" for completeness.
BTW, how can the old controller role work without supplying VALIDATOR_KEY. Does it also rely on an old version of SagaOS which doesn't require it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Old version of controller only runs pre-CCV SagaOS (so the one currently in mainnet), which uses the mnemonic instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be addressed
| {{- if not .ValidatorKey }} | ||
| - containerPort: 26658 | ||
| name: privval | ||
| {{- end }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{% if remote_signer_enabled %}
- containerPort: 26658
name: privval
{% endif %}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Again, you mean controller_remote_signer_enabled ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emanuelconunaemme confirmed offline that he actually meant controller_remote_signer_enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be addressed
7fed988 to
ddcf952
Compare
ddcf952 to
244d3b8
Compare
Controller counterpart to https://github.com/sagaxyz/SagaOS/pull/227
Add conditional logic to set REMOTE_SIGNER_ENABLED and PRIV_VALIDATOR_LADDR when ValidatorKey is not supplied, for both CCV and non-CCV templates. Also expose port 26658 for the privval server when using remote signer.