Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

feat(etherscan): add caching #1108

Merged
merged 14 commits into from
Apr 6, 2022
Merged

Conversation

onbjerg
Copy link
Collaborator

@onbjerg onbjerg commented Apr 4, 2022

Supersedes #935

@onbjerg onbjerg changed the title Feat/etherscan cache feat(etherscan): add caching Apr 4, 2022
ethers-etherscan/src/lib.rs Outdated Show resolved Hide resolved
@onbjerg onbjerg marked this pull request as ready for review April 4, 2022 16:14
@onbjerg
Copy link
Collaborator Author

onbjerg commented Apr 4, 2022

This should work now / it works when testing locally on mds1/drai

@onbjerg
Copy link
Collaborator Author

onbjerg commented Apr 4, 2022

Actually, I should probably add an actual error for rate limits so we can catch it in Foundry for retries

@onbjerg
Copy link
Collaborator Author

onbjerg commented Apr 4, 2022

This fails when the file doesn't exist, going back in draft

@onbjerg onbjerg marked this pull request as draft April 4, 2022 17:15
@onbjerg onbjerg marked this pull request as ready for review April 4, 2022 18:13
ethers-etherscan/src/contract.rs Outdated Show resolved Hide resolved
@gakonst
Copy link
Owner

gakonst commented Apr 5, 2022

  • need to change how the responses are parsed
  • then add ttl

then should be good to merge

let query = self.create_query("contract", "getabi", HashMap::from([("address", address)]));
let resp: Response<String> = self.get_json(&query).await?;
if resp.result.starts_with("Max rate limit reached") {
Copy link
Collaborator Author

@onbjerg onbjerg Apr 5, 2022

Choose a reason for hiding this comment

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

we have to dupe the check here cus it deserializes message as a string.. really annoying but no way around it really

if etherscan would give me access i would make their api better

Comment on lines 85 to 90
expiry: SystemTime::now()
.checked_add(self.ttl)
.expect("cache ttl overflowed")
.duration_since(UNIX_EPOCH)
.expect("system time is before unix epoch")
.as_secs(),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there a simpler way to do this?

let reader = std::io::BufReader::new(std::fs::File::open(path)?);
if let Ok(inner) = serde_json::from_reader::<_, CacheEnvelope<T>>(reader) {
// If this does not return None then we have passed the expiry
if SystemTime::now().checked_sub(Duration::from_secs(inner.expiry)).is_some() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same as above

@onbjerg onbjerg requested a review from gakonst April 5, 2022 23:21
@onbjerg
Copy link
Collaborator Author

onbjerg commented Apr 5, 2022

Oh wait, I re-requested review before I cached the "not verified" objects

Comment on lines +335 to +344
if let Some(ref cache) = self.cache {
// If this is None, then we have a cache miss
if let Some(src) = cache.get_source(address) {
// If this is None, then the contract is not verified
return match src {
Some(src) => Ok(src),
None => Err(EtherscanError::ContractCodeNotVerified(address)),
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is really ugly, but the alternative was to cache Result<T, EtherscanError>. Not sure. Any suggestions?

Copy link
Owner

Choose a reason for hiding this comment

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

you can prob do something like if let Some(Some(src)) = cache.get_source(address) but nbd. feel free to do in follow up

}
}

fn get<T: DeserializeOwned>(&self, prefix: &str, address: Address) -> Option<T> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Switched this to an option since we don't really care about the errors:

  • If we can't deserialize it, it's malformed and we need to rewrite it
  • If we can't open the file, the directory doesn't exist (we probably care here, should we add tracing?)

Comment on lines +335 to +344
if let Some(ref cache) = self.cache {
// If this is None, then we have a cache miss
if let Some(src) = cache.get_source(address) {
// If this is None, then the contract is not verified
return match src {
Some(src) => Ok(src),
None => Err(EtherscanError::ContractCodeNotVerified(address)),
}
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

you can prob do something like if let Some(Some(src)) = cache.get_source(address) but nbd. feel free to do in follow up

@gakonst
Copy link
Owner

gakonst commented Apr 6, 2022

Merging - let's hope it works well :)

@gakonst gakonst merged commit c436d19 into gakonst:master Apr 6, 2022
gakonst added a commit to foundry-rs/foundry that referenced this pull request Apr 6, 2022
onbjerg pushed a commit to foundry-rs/foundry that referenced this pull request Apr 6, 2022
onbjerg added a commit to foundry-rs/foundry that referenced this pull request Apr 7, 2022
* feat: etherscan identifier

* chore: bump ethers for the caching PR

gakonst/ethers-rs#1108

* feat: add cache ttl to etherscan identifier

* chore: clippy

* chore: re-add ethers patch section

* build: bump ethers

* test: fix tests

* fix: trace macros

* bump color eyre and lock tracing-subscriber (#1220)

* bump color eyre

* lock tracing-subscriber 0.3.9

* feat: pull etherscan api key and eth rpc from env

* refactor: readability in `trace.addresses`

* refactor: add `AddressIdentity` type

* refactor: add a rate limited etherscan stream

* test: don't set `ETH_RPC_URL` as its not used

* refactor: smol nit

Co-authored-by: Georgios Konstantopoulos <[email protected]>
Co-authored-by: Matthias Seitz <[email protected]>
charisma98 added a commit to charisma98/foundry that referenced this pull request Mar 4, 2023
* feat: etherscan identifier

* chore: bump ethers for the caching PR

gakonst/ethers-rs#1108

* feat: add cache ttl to etherscan identifier

* chore: clippy

* chore: re-add ethers patch section

* build: bump ethers

* test: fix tests

* fix: trace macros

* bump color eyre and lock tracing-subscriber (#1220)

* bump color eyre

* lock tracing-subscriber 0.3.9

* feat: pull etherscan api key and eth rpc from env

* refactor: readability in `trace.addresses`

* refactor: add `AddressIdentity` type

* refactor: add a rate limited etherscan stream

* test: don't set `ETH_RPC_URL` as its not used

* refactor: smol nit

Co-authored-by: Georgios Konstantopoulos <[email protected]>
Co-authored-by: Matthias Seitz <[email protected]>
0129general added a commit to 0129general/FoundryProject that referenced this pull request May 8, 2024
* feat: etherscan identifier

* chore: bump ethers for the caching PR

gakonst/ethers-rs#1108

* feat: add cache ttl to etherscan identifier

* chore: clippy

* chore: re-add ethers patch section

* build: bump ethers

* test: fix tests

* fix: trace macros

* bump color eyre and lock tracing-subscriber (#1220)

* bump color eyre

* lock tracing-subscriber 0.3.9

* feat: pull etherscan api key and eth rpc from env

* refactor: readability in `trace.addresses`

* refactor: add `AddressIdentity` type

* refactor: add a rate limited etherscan stream

* test: don't set `ETH_RPC_URL` as its not used

* refactor: smol nit

Co-authored-by: Georgios Konstantopoulos <[email protected]>
Co-authored-by: Matthias Seitz <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants