Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

allows passing key config values directly to supervisors in tests #1457

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeffgrunewald
Copy link
Contributor

Many common test setups rely on explicitly pre-generating the critical ecc key details, pubkey, ecdh function and signing function, and passing them directly to the application supervisors at startup. This change allows the miner_keys:keys/0 to accept and passthrough these test-case config overrides directly to the processes that expect them.

@@ -937,7 +937,7 @@ config_node({Miner, {TCPPort, UDPPort, JSONRPCPort}, ECDH, PubKey, _Addr, SigFun
ct_rpc:call(Miner, application, set_env, [lager, metadata_whitelist, [poc_id]]),

%% set blockchain configuration
Key = {PubKey, ECDH, SigFun},
Key = #{pubkey => PubKey, ecdh_fun => ECDH, sig_fun => SigFun},
Copy link
Contributor

@andymck andymck Mar 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure no side effects of the Key format changing to a map ? ie other consumers of application:get_env(blockchain, key)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can't find any; we only ever set that value one time (in the config_node/2 in the ct_utils setup) and otherwise it's whatever is in sys.config.

this only place i can see this will cause problems is here which you would only encounter if you're trying to inject pre-built pubkey/signing functions and enabling the embedded gateway-rs and since gateway-rs is supposed to handle all key stuffs when it's turned on i'm not sure that's a valid scenario to worry about anyway

@andymck
Copy link
Contributor

andymck commented Mar 1, 2022

Confirming my POC test suite is working again with this change

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants