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

check status of peers in kdf network #25

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan added the enhancement New feature or request label Sep 3, 2024
@onur-ozkan onur-ozkan marked this pull request as ready for review September 9, 2024 08:31
Copy link
Member

@laruh laruh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! first review iteration

src/net/rpc.rs Outdated Show resolved Hide resolved
/// WARNING: This is designed for performance-oriented use-cases utilizing `FxHashMap`
/// under the hood and is not suitable for cryptographic purposes.
#[derive(Clone, Debug)]
pub struct ExpirableMap<K: Eq + Hash, V>(FxHashMap<K, ExpirableEntry<V>>);
Copy link
Member

Choose a reason for hiding this comment

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

ExpirableMap object and its methods are duplicated in KomodoDeFi repo. https://github.com/KomodoPlatform/komodo-defi-framework/blob/14eeaeeb3da0745f7f4af1bf241e9c2d8755bc50/mm2src/common/expirable_map.rs

Can we have it as a dep, like you did with proxy sig?

proxy_signature = { git = "https://github.com/KomodoPlatform/komodo-defi-framework", rev = "9ebc006" }

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want to add the large common crate from KDF just to use this simple module.

README.md Outdated Show resolved Hide resolved
Signed-off-by: onur-ozkan <[email protected]>

### Architecture (TODO: OUTDATED)
### Architecture
Copy link

Choose a reason for hiding this comment

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

Could you plz add some intro to the README about how this service is used?
Is it basically for KDF app to access backends (web3, quicknode etc providers)?
Will it be a network of proxies or each KDF peer will have its own proxy locally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you plz add some intro to the README about how this service is used?

Sure.

Is it basically for KDF app to access backends (web3, quicknode etc providers)?

Yes, we use this to hide the keys for our paid services and protect them from misuse. You can take a look on https://gist.github.com/onur-ozkan/725f56c05a2eb01e0428f03191f8da86 for some more context.

Will it be a network of proxies or each KDF peer will have its own proxy locally?

No.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you plz add some intro to the README about how this service is used?

Done that.

src/proxy/http/mod.rs Outdated Show resolved Hide resolved
Signed-off-by: onur-ozkan <[email protected]>
@laruh
Copy link
Member

laruh commented Sep 26, 2024

@onur-ozkan
Copy link
Member Author

@onur-ozkan nightly lint is failing https://github.com/KomodoPlatform/komodo-defi-proxy/actions/runs/11048571940/job/30692339346?pr=25

Yeah, nightly clippy is broken.

Copy link
Member

@laruh laruh left a comment

Choose a reason for hiding this comment

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

LGTM!

@shamardy
Copy link

@onur-ozkan you can merge this PR now :)

@onur-ozkan
Copy link
Member Author

@onur-ozkan you can merge this PR now :)

KomodoPlatform/komodo-defi-framework#2194 is kind a blocker for this PR. Due to some recent breaking changes in that PR, I had to do this change in this PR. Same can happen again, so I will wait until we merge KomodoPlatform/komodo-defi-framework#2194.

@laruh
Copy link
Member

laruh commented Oct 1, 2024

@onur-ozkan in the proxy repo could we use some expirable map improvements provided in this discussion thread KomodoPlatform/komodo-defi-framework#2232 (comment) ?

@onur-ozkan
Copy link
Member Author

@onur-ozkan in the proxy repo could we use some expirable map improvements provided in this discussion thread KomodoPlatform/komodo-defi-framework#2232 (comment) ?

I will, waiting for final result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants