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

Kill bad tokens 1 #3156

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Kill bad tokens 1 #3156

wants to merge 6 commits into from

Conversation

m-lord-renkse
Copy link
Contributor

Description

TBD

Changes

TBD

How to test

TBD

Fixes
TBD

@@ -42,7 +42,7 @@ pub struct Detector {
hardcoded: HashMap<eth::TokenAddress, Quality>,
/// cache which is shared and updated by multiple bad token detection
/// mechanisms
cache: Cache,
cache: Arc<Cache>,
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 am not very happy with having this in an Arc when Detector has to be wrapped in an Arc too. I didn't have time to think of a better solution. The cache has to be Arced because it has to be shared among solvers, and the Detector has to be wrapped in an Arc so we can box it in order to pass it as a future.

}

impl Detector {
pub fn with_config(mut self, config: HashMap<eth::TokenAddress, Quality>) -> Self {
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 am up for a DetectorBuilder.

pub struct BadTokenDetector {
/// Whether or not the bad token detector is enabled
#[serde(default = "bool::default")]
pub enabled: bool,
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 am not super convinced about this. But I guess we should have something like this in order to enable it without any token lists configured.

}
}

impl Cache {
/// Creates a new instance which evicts cached values after a period of
/// time.
pub fn new(max_age: Duration, max_size: usize) -> Self {
pub fn new(bad_token_detection_cache: &BadTokenDetectionCache) -> Self {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe better to implement Copy trait and remove the pass by reference?

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.

2 participants