feat(sns): Add list and add-sns-wasm-for-tests command
#139
Conversation
extensions/sns/src/utils.rs
Outdated
| fn get_agent(ic_url: &str) -> Result<Agent> { | ||
| Agent::builder() | ||
| .with_url(ic_url) | ||
| .with_verify_query_signatures(false) |
There was a problem hiding this comment.
I'm not really sure what this does, but it looks scary, like leaving your house without locking the door. Sure, if there are no evil doers around, everything works just fine. But you still wouldn't do that.
| } | ||
| }; | ||
|
|
||
| let agent = utils::get_mainnet_agent()?; |
There was a problem hiding this comment.
NCR
IIUC, this is only used in one of the branches, specifically, List. Perhaps the list::exec can built agent for itself?
There was a problem hiding this comment.
Hmm, that's tricky because list::exec in the future won't have the necessary information to create the agent
| SnsLibSubCommand::DeployTestflight(args) => deploy_testflight(args), | ||
| SnsLibSubCommand::InitConfigFile(args) => init_config_file::exec(args), | ||
| SnsLibSubCommand::PrepareCanisters(args) => prepare_canisters::exec(args), | ||
| SnsLibSubCommand::Propose(args) => propose::exec(args), | ||
| SnsLibSubCommand::NeuronIdToCandidSubaccount(args) => { | ||
| neuron_id_to_candid_subaccount::exec(args) | ||
| } | ||
| }?; | ||
| Ok(()) | ||
| SnsLibSubCommand::AddSnsWasmForTests(args) => add_sns_wasm_for_tests(args), | ||
| SnsLibSubCommand::List(args) => list::exec(args, &agent).await, |
There was a problem hiding this comment.
NCR
Would be nice if all these looked the same. I.e.
X::Y(y) => y::exec(y),
Even better (but more work, and therefore, not recommended now, but next time):
subcommand.unwrap().exec()
where unwrap returns a Box<dyn Exec>, and Exec is a new trait that requires fn exec(&self) -> Result<...>;, and all the args types implement it.
There was a problem hiding this comment.
Unfortunately this difference is an annoying consequence of the scoping of add_sns_wasm_for_tests, which can't be fixed in this PR as it's determined by the monorepo :(
anchpop
force-pushed
the
@anchpop/update-sns-extension-with-new-commands
branch
4 times, most recently
from
fa4a856 to
a7f999a
Compare
anchpop
force-pushed
the
@anchpop/update-sns-extension-with-new-commands
branch
2 times, most recently
from
31e9a4a to
fc0789f
Compare
anchpop
force-pushed
the
@anchpop/update-sns-extension-with-new-commands
branch
from
fc0789f to
02a6247
Compare
This brings it in sync with the monorepo's sns cli.
add-sns-wasm-for-testsis hidden as it's relatively "developer only" and I don't have a good idea of when it's needed, other than that I know snsdemo uses it.The commands were implemented in the monorepo, and are just called from here.