Skip to content
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

Set explorer_url in private Tangle example #454

Merged
merged 3 commits into from
Oct 26, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions examples/account/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ async fn main() -> Result<()> {
// As an example we are treating the devnet as a `private-tangle`, so we use `dev`.
// Replace this with `tangle` if you run this against a one-click private tangle.
let network_name = "dev";
let network = Network::try_from_name(network_name)?;
let mut network = Network::try_from_name(network_name)?;

// If you deployed an explorer, set its url here.
network.set_explorer_url("http://127.0.0.1:8082/".parse()?)?;

// In a locally running one-click tangle, this would often be `http://127.0.0.1:14265/`
let private_node_url = "https://api.lb-0.h.chrysalis-devnet.iota.cafe";
Copy link
Contributor

@cycraig cycraig Oct 26, 2021

Choose a reason for hiding this comment

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

The explorer URL does not match the default node URL in this example, does that matter?

Maybe specify the default explorer URL for the devnet and add the example for the private Tangle case in the comment, as per private_node_url?

E.g.

  // If you deployed an explorer locally this would usually be `http://127.0.0.1:8082/identity-resolver`
  network.set_explorer_url("https://explorer.iota.org/devnet/identity-resolver".parse()?)?;

On a related note, does private_node_url use the deprecated devnet URL intentionally so publishing fails?
I was looking at old code, devnet replaced testnet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, makes more sense that way.

I agree that the url should be moved out of the Network, but I don't really know where. When we do that, we should think about renaming the explorer url to resolver url, now that we no longer point to the actual explorer? Would be a bit more accurate.

Copy link
Contributor

@cycraig cycraig Oct 26, 2021

Choose a reason for hiding this comment

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

The identity-resolver is still part of the explorer so I'm not too bothered by the current name. The only concern I have with the resolverUrl name is that it might conflict with the actual Resolver API.

We could have a newtype wrapper---ExplorerUrl/ResolverUrl---around a Url that offers methods to create it yourself or from an existing network.

E.g. something like

fn new(url: Url) -> Self;
fn try_from_network(network: &Network) -> Result<Self>; // only returns for the well-known main & devnet networks

fn message_url(message_id: &str) -> Url;  // redirects to the message viewer
fn identity_url/resolver_url(did: &IotaDID) -> Url; // redirects to the identity-resolver

and still implement FromStr etc. In this case I would keep the name as ExplorerUrl since there's a dedicated method to append "identity-resolver" to it.

This may be over-engineering it, however.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's one way to do it, and I'm not opposed to it. As you say, though, it might be over-engineering it, since it's really just a URL. A very simple solution would be to export const or static Urls for main and devnet, and everything else is up to the app developer.


Expand All @@ -46,7 +50,7 @@ async fn main() -> Result<()> {
})
// Configure a client for the private network, here `dev`
// Also set the URL that points to the REST API of the node
.client(network, |builder| {
.client(network.clone(), |builder| {
// unwrap is safe, we provided a valid node URL
builder.node(private_node_url).unwrap()
})
Expand All @@ -71,7 +75,7 @@ async fn main() -> Result<()> {
// Prints the Identity Resolver Explorer URL, the entire history can be observed on this page by "Loading History".
println!(
"[Example] Explore the DID Document = {}/{}",
iota_did.network()?.explorer_url().unwrap().to_string(),
network.explorer_url().expect("explorer url was set").to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
network.explorer_url().expect("explorer url was set").to_string(),
network.explorer_url().expect("no explorer url set").to_string(),

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've never quite understood how expect is supposed to be used. You can read it as "expect explorer url was set" which makes sense when read in code, but it will panic with "panicked with: explorer url was set", which would be the opposite of what happened. And getting both right seems to be mutually exclusive. Anyway... I'll change it to your suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I always read expect as "I expect that if this panics, this is the reason why", or "if this panics I expect this to be the reason".

I rate the error messages are more important for debugging when it actually panics, so I try write them for that.

iota_did.to_string()
);

Expand Down