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

chore(client): remove price tolerance at the client #824

Merged
merged 12 commits into from
Oct 18, 2023

Conversation

joshuef
Copy link
Contributor

@joshuef joshuef commented Oct 11, 2023

This should no longer be needed with repayments reusing existing payments
This also removes some client indirection## Description

Summary generated by Reviewpad on 11 Oct 23 07:55 UTC

This pull request includes two patches.

The first patch removes the price tolerance at the client and some client indirection. The get_store_costs_at_address method is removed from the Client struct, as it is no longer needed with repayments reusing existing payments.

The second patch ensures that only the closest nodes are used for pricing. The get_closest_peers method now limits the number of close nodes to CLOSE_GROUP_SIZE and sorts the close nodes based on their distance to the record address before truncating the list. This ensures that only the closest nodes are used for pricing calculations.

@reviewpad reviewpad bot requested a review from maqi October 11, 2023 07:55
@reviewpad reviewpad bot added Small Pull request is small waiting-for-review labels Oct 11, 2023
@joshuef joshuef force-pushed the RemoveAPIIndirectionAtClient branch from ddd0f45 to f05d0a4 Compare October 11, 2023 11:47
@bochaco bochaco force-pushed the RemoveAPIIndirectionAtClient branch from f05d0a4 to 9e356b3 Compare October 11, 2023 12:32
@bochaco bochaco enabled auto-merge October 11, 2023 12:33
@joshuef joshuef force-pushed the RemoveAPIIndirectionAtClient branch from 9e356b3 to f01202c Compare October 16, 2023 10:29
@reviewpad reviewpad bot added Medium Medium sized PR and removed Small Pull request is small labels Oct 16, 2023
@joshuef joshuef force-pushed the RemoveAPIIndirectionAtClient branch 3 times, most recently from 71b60c0 to ac4094e Compare October 17, 2023 10:13
@reviewpad reviewpad bot added Large Large sized PR and removed Medium Medium sized PR labels Oct 17, 2023
@joshuef joshuef force-pushed the RemoveAPIIndirectionAtClient branch 5 times, most recently from a7d16ee to a0aa382 Compare October 17, 2023 14:21
Ok(r) => r,
// any error here will result in a repayment of the register
// TODO: be smart about this and only pay for storage if we need to

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
@joshuef joshuef force-pushed the RemoveAPIIndirectionAtClient branch 2 times, most recently from f442278 to 7b77c7b Compare October 17, 2023 18:57
This should no longer be needed with repayments reusing existing payments
This also removes some client indirection
Remove payment which makes upload valid, instead only
pay 0 tokens and that's it.
BREAKING CHANGE: updates client register creation APIs
@joshuef joshuef force-pushed the RemoveAPIIndirectionAtClient branch from bc0cb8d to 7ceb986 Compare October 18, 2023 08:25
@reviewpad
Copy link

reviewpad bot commented Oct 18, 2023

Reviewpad Report

‼️ Errors

  • Unconventional commit detected: 'Revert "feat: keep transfers in mem instead of mem and i/o heavy cashnotes"

This reverts commit 9eb32bb.' (102afc8)

@@ -205,6 +207,8 @@
all_data_payments: BTreeMap<XorName, Vec<(MainPubkey, NanoTokens)>>,
verify_store: bool,
) -> WalletResult<NanoTokens> {
// TODO:

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
@bochaco bochaco added this pull request to the merge queue Oct 18, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 18, 2023
@joshuef joshuef merged commit 72a41ea into maidsafe:main Oct 18, 2023
28 checks passed
@joshuef joshuef deleted the RemoveAPIIndirectionAtClient branch October 18, 2023 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Large Large sized PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants