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

feat: add cache to registry #256

Merged
merged 23 commits into from
Aug 29, 2023
Merged

feat: add cache to registry #256

merged 23 commits into from
Aug 29, 2023

Conversation

marshacb
Copy link
Contributor

@marshacb marshacb commented Aug 23, 2023

  • adds cache to the registry
  • caches queried assets, foreign assets, and pool assets
  • defaults to reading from the cache, followed by the assets registry and chain state

closes: #225

@marshacb marshacb changed the title [WIP]: add cache to registry add cache to registry Aug 23, 2023
@TarikGul
Copy link
Member

From an initial look. I think the registry should have implemented getters and setters that either look for a given item in the cache, or set an item in the cache.

For example:

class Registry {
    constructor() {
        ....
        this.cache = {
            ...
        }
    }
    
    public cacheLookupAsset(assetId: string) {
        
    }
    
    public cacheAddAsset(assetId: string) {
    
    }
}

NOTE: We already know the specname, and the relaychain so we dont need those values as args.

@marshacb
Copy link
Contributor Author

From an initial look. I think the registry should have implemented getters and setters that either look for a given item in the cache, or set an item in the cache.

For example:

class Registry {
    constructor() {
        ....
        this.cache = {
            ...
        }
    }
    
    public cacheLookupAsset(assetId: string) {
        
    }
    
    public cacheAddAsset(assetId: string) {
    
    }
}

NOTE: We already know the specname, and the relaychain so we dont need those values as args.

great idea! Ill modify this to reflect the getter setter structure

@TarikGul
Copy link
Member

Some additional thoughts.

  1. For setting items in the cache since we have specific types it would either need to take in a second arg called type (or something like it) that tells us whether the assetId is a LP token, FA token, or just regular asset part of the assets pallet.

OR

  1. We can have multiple methods, setForeignAssetInCache, setLiquidPoolTokenInCache, etc. That way each assetId can be a different format>

OR

  1. We can be more generic for (2), and have a single function that has two args:
type GenericAsset<T> = T extends 'foreignAsset'
	? <some_type>
	: T extends 'liquidPools'
	? <some_type>
	: T extends 'assetpallet'
	? <some_type>
	: never;
	
...
public setAssetIdInCache<T>(asset: GenericAsset<T>, type: T)
...

@marshacb marshacb requested a review from TarikGul August 24, 2023 03:26
src/registry/Registry.ts Outdated Show resolved Hide resolved
src/registry/Registry.ts Outdated Show resolved Hide resolved
src/registry/Registry.ts Outdated Show resolved Hide resolved
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

Just some nits to address on the above. But overall really solid PR!

@marshacb marshacb requested a review from TarikGul August 24, 2023 19:06
@marshacb marshacb changed the title add cache to registry feat: add cache to registry Aug 24, 2023
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

Some small nits, but really amazing job!

@marshacb marshacb merged commit 8836821 into main Aug 29, 2023
6 checks passed
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.

The API should query for tokens that are not in the Registry, then if they exist cache them.
2 participants