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

Scx1332/tokenrefaktor #3042

Merged
merged 6 commits into from
Feb 6, 2024
Merged

Scx1332/tokenrefaktor #3042

merged 6 commits into from
Feb 6, 2024

Conversation

scx1332
Copy link
Collaborator

@scx1332 scx1332 commented Feb 5, 2024

No description provided.

@scx1332 scx1332 requested a review from a team as a code owner February 5, 2024 12:52
@scx1332 scx1332 linked an issue Feb 5, 2024 that may be closed by this pull request
Copy link
Contributor

@nieznanysprawiciel nieznanysprawiciel left a comment

Choose a reason for hiding this comment

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

I believe this code could still be much more simpler than it is, but I'm not going to push further for now. I left a few comments, please look if you would like to apply them

Comment on lines +46 to +49
impl Display for TokenName {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.0)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on lines +247 to +251
impl Display for PaymentPlatformTriple {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}-{}-{}", self.driver, self.network, self.token)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

core/payment/src/api/allocations.rs Outdated Show resolved Hide resolved
core/payment/src/api/allocations.rs Outdated Show resolved Hide resolved
Comment on lines 123 to 155
let default_platform = Self::default_testnet();
log::debug!("Empty paymentPlatform object, using {default_platform}");
default_platform
} else if p.token.is_some() && p.network.is_none() && p.driver.is_none() {
let token = p.token.as_ref().unwrap();
if token == "GLM" || token == "tGLM" {
return Err(bad_req_and_log(format!(
"Uppercase token names are not supported. Use lowercase glm or tglm instead of {}",
token
)));
} else if token == "glm" {
let default_platform = Self::default_mainnet();
log::debug!("Selected network {default_platform} (default for glm token)");
default_platform
} else if token == "tglm" {
let default_platform = Self::default_testnet();
log::debug!("Selected network {default_platform} (default for tglm token)");
default_platform
} else {
return Err(bad_req_and_log(format!(
"Only glm or tglm token values are accepted vs {token} provided"
)));
}
} else {
let network_str = p.network.as_deref().unwrap_or_else(|| {
if let Some(token) = p.token.as_ref() {
if token == "glm" {
log::debug!(
"Network not specified, using default {}, because token set to glm",
DEFAULT_MAINNET_NETWORK
);
DEFAULT_MAINNET_NETWORK.into()
} else {
log::debug!(
"Network not specified, using default {}",
DEFAULT_TESTNET_NETWORK
);
DEFAULT_TESTNET_NETWORK.into()
}
} else {
log::debug!(
"Network not specified and token not specified, using default {}",
DEFAULT_TESTNET_NETWORK
);
DEFAULT_TESTNET_NETWORK.into()
}
});
let network = validate_network(network_str).map_err(|err| {
bad_req_and_log(format!("Validate network failed (1): {err}"))
})?;

let driver_str = p.driver.as_deref().unwrap_or_else(|| {
log::debug!(
"Network not specified, using default {}, because token set to glm",
DEFAULT_MAINNET_NETWORK
"Driver not specified, using default {}",
DEFAULT_PAYMENT_DRIVER
);
DEFAULT_MAINNET_NETWORK.into()
DEFAULT_PAYMENT_DRIVER.into()
});
let driver = validate_driver(&network, driver_str)
.map_err(|err| bad_req_and_log(format!("Validate driver failed (1): {err}")))?;

if let Some(token) = p.token.as_ref() {
let token =
TokenName::from_token_string(&driver, &network, token).map_err(|err| {
bad_req_and_log(format!("Validate token failed (1): {err}"))
})?;
log::debug!("Selected network {}-{}-{}", driver, network, token);
Self {
driver,
network,
token,
}
} else {
let default_token = TokenName::default(&driver, &network);

log::debug!(
"Network not specified, using default {}",
DEFAULT_TESTNET_NETWORK
"Selected network with default token {}-{}-{}",
driver,
network,
default_token
);
DEFAULT_TESTNET_NETWORK.into()
Self {
driver,
network,
token: default_token,
}
}
} else {
log::debug!(
"Network not specified and token not specified, using default {}",
DEFAULT_TESTNET_NETWORK
);
DEFAULT_TESTNET_NETWORK.into()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be match statement probably:

match (p.driver, p.network, p.token) {
    (None, None, None) => { ... },
    (None, None, Some(token)) => { ... }
}

Comment on lines 129 to 134
if token == "GLM" || token == "tGLM" {
return Err(bad_req_and_log(format!(
"Uppercase token names are not supported. Use lowercase glm or tglm instead of {}",
token
)));
} else if token == "glm" {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks similar to from_token_string function implementation


let network = validate_network(network_str)
.map_err(|err| bad_req_and_log(format!("Validate network failed (2): {err}")))?;
fn validate_network(network: &str) -> Result<NetworkName, String> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Common practice would be to use anyhow if you need to return something generic as string.

scx1332 and others added 3 commits February 5, 2024 18:27
Co-authored-by: nieznanysprawiciel <nieznany.sprawiciel@gmail.com>
Co-authored-by: nieznanysprawiciel <nieznany.sprawiciel@gmail.com>
@scx1332 scx1332 merged commit 11e8009 into release/v0.15 Feb 6, 2024
27 checks passed
@scx1332 scx1332 deleted the scx1332/tokenrefaktor branch February 6, 2024 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Driver & Network Selection - new mechanism
2 participants