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

Dynamic payment schema for polygon #1787

Merged
merged 10 commits into from
Jan 10, 2022

Conversation

scx1332
Copy link
Collaborator

@scx1332 scx1332 commented Jan 4, 2022

No description provided.

@scx1332 scx1332 requested a review from a team January 4, 2022 15:05
@nieznanysprawiciel nieznanysprawiciel self-requested a review January 5, 2022 11:58
@tworec
Copy link
Contributor

tworec commented Jan 5, 2022

@scx1332 it would be great if you can describe the strategy introduced within this PR in few sentences (currently description is void, and it took me several minutes to get to the point)

@scx1332 scx1332 changed the base branch from master to release/v0.9 January 5, 2022 12:27
@prekucki prekucki self-requested a review January 5, 2022 12:36
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.

If we need to execute different code for each network, than probably we should make an abstraction (dyn Trait) for this.

But this is not blocking comment.

core/payment-driver/erc20/src/erc20/wallet.rs Outdated Show resolved Hide resolved
@@ -233,10 +260,15 @@ pub async fn send_transactions(
match serde_json::from_str::<YagnaRawTransaction>(&tx.encoded) {
Ok(raw_tx) => raw_tx,
Err(err) => {
let error = format!("JSON parse failed, unrecoverable error: {}", err);
log::error!("Error during serialization of json: {:?}", err);
Copy link
Contributor

Choose a reason for hiding this comment

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

This log has no context, where it happened. It will be hard to understand

@nieznanysprawiciel nieznanysprawiciel self-requested a review January 5, 2022 13:27
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.

Please check my comments, before I will approve

@nieznanysprawiciel nieznanysprawiciel self-requested a review January 5, 2022 13:28
prekucki
prekucki previously approved these changes Jan 5, 2022
@maaktweluit
Copy link
Contributor

we are getting a lof of settings around gas.. would be nice to make it simpler instead of adding layers on top of layers of fixes

@scx1332 scx1332 dismissed stale reviews from nieznanysprawiciel and prekucki via 1e19c5c January 5, 2022 15:49
@nieznanysprawiciel nieznanysprawiciel merged commit 8d2b524 into release/v0.9 Jan 10, 2022
@nieznanysprawiciel nieznanysprawiciel deleted the scx1332/maticpayments branch January 10, 2022 12:28
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.

5 participants