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

improve: leverage default TTL & leverage caching #938

Merged
merged 2 commits into from
Sep 16, 2023

Conversation

james-a-morris
Copy link
Contributor

This PR enabled the ability for the hubpool client to utilize caching. Additionally, this change sets the RedisClient to use the default 2 week TTL if not provided.

@@ -67,7 +67,7 @@ export class RedisCache implements interfaces.CachingMechanismInterface {
await this.instantiate();
}
// Call the setRedisKey function to set the value in redis.
await setRedisKey(key, JSON.stringify(value), this.redisClient, ttl);
await setRedisKey(key, JSON.stringify(value), this.redisClient, ttl ?? constants.DEFAULT_CACHING_TTL);
Copy link
Member

Choose a reason for hiding this comment

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

why not set ttl = constants.DEFAULT_CACHING_TTL in the function declaration?

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'm not super opinionated on this. I wanted to keep the function signatures as close to the interface as possible. However, I like this approach because if we ever decide to expand on the logic we only need to define the default once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

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

One comment.

@james-a-morris james-a-morris merged commit 3b4c101 into master Sep 16, 2023
2 checks passed
nicholaspai pushed a commit that referenced this pull request Sep 20, 2023
* improve: leverage default TTL

* improve: set fn signature to default
nicholaspai added a commit that referenced this pull request Sep 28, 2023
…tamps (#944)

* improve(dataworker): Remove `transferThreshold`

Fixes ACX-1537

Removes a lot of code needed to support deprecated config store variable that is always set to 0 in production

* Update Dataworker.ts

* bumpity

* Update Dataworker.ts

* Update package.json

* Update yarn.lock

* wip

* fix(Dataworker): Dereference originChainId by index, not key (#936)

* fix(Dataworker): Dereference originChainId by index, not key

Temporary fix to resolve an issue with future quoteTimestamps. We need a
more robust solution than this in the long run.

* Extract originToken from array

* lint

* WIP

Signed-off-by: Matt Rice <[email protected]>

---------

Signed-off-by: Matt Rice <[email protected]>
Co-authored-by: Matt Rice <[email protected]>

* improve: leverage default TTL & leverage caching (#938)

* improve: leverage default TTL

* improve: set fn signature to default

* feat: add dataworker force bundle range (#912)

* feat: add dataworker force bundle range

* docs: add documentation to example env

* improve: allow for boba

* improve: allow forced proposal range with sending

---------

Co-authored-by: Paul <[email protected]>

* feat: test chain routes (#897)

* feat: Script for automating SpokePool deposits

This is useful for making deposits to the zkSync SpokePool(s) because
zkSync's block explorer doesn't yet support making transactions via
proxy contracts.

* Add yarn script target + tweak usage

* Add basic support for dumping SpokePool config

* lint

* fix: Gracefully handle token searches where token missing

Not all tokens are defined on all chains, so don't try to drop case
unless the token actually exists on a chain.

* Add "fetch" for dumping deposit and fill information

Currently limitations:
 - Does not display updated values (i.e. after a speed-up).
 - Does not display message data.
 - Should normalise from the token decimals for better readability.
 - Does not gracefully handle when the txnHash is not found.

* lint

* fix conflict

* lint

* chore: allow base-units to easily be sent

* chore: allow token to be passed as origin token addr or symbol

* feat: test chain routes

* nit: improve docs

* docs: add docs

* improve: account for checksummed value

Co-authored-by: Paul <[email protected]>

* improve: remove unneeded flag

Co-authored-by: Paul <[email protected]>

* improve: docs

Co-authored-by: Paul <[email protected]>

* improve: do not hardcode amounts

* improve: modification

* improve: dynamically compute price

---------

Co-authored-by: Paul <[email protected]>
Co-authored-by: nicholaspai <[email protected]>

* WIP

* Update yarn.lock

* Revert "WIP"

This reverts commit 381e5bf.

* feat: Add protection against computing LP fees for future quote timestamps

This resolves the [reverted SDK PR](across-protocol/sdk#399) and re-introduces protection from deposit quote timestamps that are impossible to compute.

This PR essentially moves that protection to the production version of Hub and Spoke clients so that the unit tests can continue to work and not have to add additional logic to set quote times correctly. Ideally in the future we set the quote times how they would in production but for now this solution results in a much smaller code diff

* lint

* fix

* lint

* Update HubPoolClient.ts

* refactor: Import getNetworkName from sdk (#945)

* chore: bump sdk to latest (#943)

* chore: bump sdk to latest

* chore: bump version

* chore: bump sdk

---------

Signed-off-by: Matt Rice <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: Matt Rice <[email protected]>
Co-authored-by: James Morris, MS <[email protected]>
nicholaspai added a commit that referenced this pull request Sep 29, 2023
* improve(dataworker): Remove `transferThreshold`

Fixes ACX-1537

Removes a lot of code needed to support deprecated config store variable that is always set to 0 in production

* Update Dataworker.ts

* Update Dataworker.ts

* Update package.json

* Update yarn.lock

* wip

* WIP

* Update yarn.lock

* Revert "WIP"

This reverts commit 381e5bf.

* chore: bump sdk to latest (#943)

* chore: bump sdk to latest

* chore: bump version

* chore: bump sdk

* Update src/dataworker/Dataworker.ts

Co-authored-by: Paul <[email protected]>

* feat: Add protection against computing LP fees for future quote timestamps (#944)

* improve(dataworker): Remove `transferThreshold`

Fixes ACX-1537

Removes a lot of code needed to support deprecated config store variable that is always set to 0 in production

* Update Dataworker.ts

* bumpity

* Update Dataworker.ts

* Update package.json

* Update yarn.lock

* wip

* fix(Dataworker): Dereference originChainId by index, not key (#936)

* fix(Dataworker): Dereference originChainId by index, not key

Temporary fix to resolve an issue with future quoteTimestamps. We need a
more robust solution than this in the long run.

* Extract originToken from array

* lint

* WIP

Signed-off-by: Matt Rice <[email protected]>

---------

Signed-off-by: Matt Rice <[email protected]>
Co-authored-by: Matt Rice <[email protected]>

* improve: leverage default TTL & leverage caching (#938)

* improve: leverage default TTL

* improve: set fn signature to default

* feat: add dataworker force bundle range (#912)

* feat: add dataworker force bundle range

* docs: add documentation to example env

* improve: allow for boba

* improve: allow forced proposal range with sending

---------

Co-authored-by: Paul <[email protected]>

* feat: test chain routes (#897)

* feat: Script for automating SpokePool deposits

This is useful for making deposits to the zkSync SpokePool(s) because
zkSync's block explorer doesn't yet support making transactions via
proxy contracts.

* Add yarn script target + tweak usage

* Add basic support for dumping SpokePool config

* lint

* fix: Gracefully handle token searches where token missing

Not all tokens are defined on all chains, so don't try to drop case
unless the token actually exists on a chain.

* Add "fetch" for dumping deposit and fill information

Currently limitations:
 - Does not display updated values (i.e. after a speed-up).
 - Does not display message data.
 - Should normalise from the token decimals for better readability.
 - Does not gracefully handle when the txnHash is not found.

* lint

* fix conflict

* lint

* chore: allow base-units to easily be sent

* chore: allow token to be passed as origin token addr or symbol

* feat: test chain routes

* nit: improve docs

* docs: add docs

* improve: account for checksummed value

Co-authored-by: Paul <[email protected]>

* improve: remove unneeded flag

Co-authored-by: Paul <[email protected]>

* improve: docs

Co-authored-by: Paul <[email protected]>

* improve: do not hardcode amounts

* improve: modification

* improve: dynamically compute price

---------

Co-authored-by: Paul <[email protected]>
Co-authored-by: nicholaspai <[email protected]>

* WIP

* Update yarn.lock

* Revert "WIP"

This reverts commit 381e5bf.

* feat: Add protection against computing LP fees for future quote timestamps

This resolves the [reverted SDK PR](across-protocol/sdk#399) and re-introduces protection from deposit quote timestamps that are impossible to compute.

This PR essentially moves that protection to the production version of Hub and Spoke clients so that the unit tests can continue to work and not have to add additional logic to set quote times correctly. Ideally in the future we set the quote times how they would in production but for now this solution results in a much smaller code diff

* lint

* fix

* lint

* Update HubPoolClient.ts

* refactor: Import getNetworkName from sdk (#945)

* chore: bump sdk to latest (#943)

* chore: bump sdk to latest

* chore: bump version

* chore: bump sdk

---------

Signed-off-by: Matt Rice <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: Matt Rice <[email protected]>
Co-authored-by: James Morris, MS <[email protected]>

---------

Signed-off-by: Matt Rice <[email protected]>
Co-authored-by: James Morris, MS <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: Matt Rice <[email protected]>
@pxrl pxrl deleted the james/add-default-caching-ttl branch September 5, 2024 10:32
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