feat: optional signing providers - #2295
Conversation
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
…to register Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
…tional-signing-providers Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
…tional-signing-providers Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com> # Conflicts: # wallet-gateway/remote/src/user-api/rpc-gen/index.ts
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
| } | ||
|
|
||
| const hasSigningDb = process.argv.slice(2).includes('signing-db') | ||
| // TODO check if I can get rid db and signing-db arguments. |
There was a problem hiding this comment.
@mjuchli-da Do you think we can remove arguments 'db' and 'signing-db'? If not, then I will just remove TODO comment.
…ith deprecation warning Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
…tional-signing-providers Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
…gs that get added as fallback by zod.preprocess in 'required' Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
mjuchli-da
left a comment
There was a problem hiding this comment.
Thanks! It looks good but let us investigate if we can simplify the configuration as outlined in the comment below.
| } | ||
|
|
||
| const hasSigningDb = process.argv.slice(2).includes('signing-db') | ||
| // TODO check if I can get rid db and signing-db arguments. |
| ), | ||
| }) | ||
|
|
||
| const providerEnableSchema = z.object({ |
There was a problem hiding this comment.
I actually think we might not need this: what if we just assume that if the config holds a record for a signing provider, then the intention is to enable it. That means, by default, everything is disabled, but if the property fireblocks with all its fields exists, then we plan to enable the driver.
This should also work fine for walletKernel and participant which do not hold sub-fields – here we can use the ~ tilde symbol as per yaml standard.
The reason why I think we should at least check if this is doable is because it might make the subsequent plug&play of signing drivers more user friendly.
There was a problem hiding this comment.
We could just assume that lack of provider's object in config means it won't register, but it is a breaking change that would require changing config after updating to Gateway version that will have this feature.
Explicit opt-out is one of two ways of preventing breaking changes I implemented, and the second is in case of missing config properties, like signingProviders.blockdaemon.baseUrl, to fallback on environment variables that were used before.
I see 3 options depending on how we want to balance no breaking changes vs simpler config and easier plug&play:
- Keep as is. No breaking changes, more complexity.
- Remove
enablefield, but keep checking environment variables as fallback for values that are now supposed to be in WG config. i.e if you have all env vars needed to register blockdaemon but nosigningProviders.blockdaemonin config, it still registers. Onlywallet-kernelandparticipanthave breaking changes, simpler config for users, but still a bit convoluted for us to keep supporting deprecated env vars. - Only existence of
signingProviders.xentry allows registration of signing driver. Simplest config and code maintenance, but absolutely requires updating config after upgrading to WG version with that feature, or no signing provider will register.
There was a problem hiding this comment.
I suggest going with option 3. With the browser extension deployment, we will anyways need to opt-in a different default setup (e.g. no postgres but extension storage instead).
There was a problem hiding this comment.
We agreed that if signingProviders is missing, we will supply a default config:
"signingProviders": {
"participant": {},
"gateway": {},
"fireblocks": {},
"blockdaemon": {},
"all the rest": {},
}
…tional-signing-providers Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
Co-authored-by: Marc Juchli <120378272+mjuchli-da@users.noreply.github.com> Signed-off-by: pawelstepien-da <pawel.stepien@digitalasset.com>
Co-authored-by: Marc Juchli <120378272+mjuchli-da@users.noreply.github.com> Signed-off-by: pawelstepien-da <pawel.stepien@digitalasset.com>
Signed-off-by: Pawel Stepien <pawel.stepien@digitalasset.com>
.