Queries #1
Queries #1
Conversation
* Extract setup of Centrifuge and TenderlyFork into context accessed via withContext * Keep virtual network alive if test fails * Export context directly and introduce debug flag to prevent vn being deleted
src/Account.ts
Outdated
| }) | ||
| } | ||
|
|
||
| transfer(to: HexString, amount: bigint) { |
There was a problem hiding this comment.
seeing a lot of versions of the transfer function still, do you want to keep these or should we evaluate and choose the best?
There was a problem hiding this comment.
removed them, they were just there as examples
src/Centrifuge.ts
Outdated
|
|
||
| const envConfig = { | ||
| mainnet: { | ||
| subqueryUrl: 'https://api.subquery.network/sq/centrifuge/pools-demo-multichain', |
There was a problem hiding this comment.
| subqueryUrl: 'https://api.subquery.network/sq/centrifuge/pools-demo-multichain', | |
| subqueryUrl: 'https://subql.embrio.tech/', |
| // Saves having to pass `account` and `chain` to every `writeContract` call | ||
| const walletClient = isLocalAccount(signer) | ||
| ? (bareWalletClient as WalletClient<any, Chain, AccountType>) | ||
| : createWalletClient({ account: address, chain, transport: custom(signer) }) |
There was a problem hiding this comment.
having a hard time understanding why the wallet client has to be recreated, from the code it looks like the wallet client could already initially be set correctly to avoid the walletClient/bareWalletClient situation. The comment is suggesting that it's being recreated because the chainid may be incorrect but it's using the same chain id as the bareWalletClient
There was a problem hiding this comment.
it's mainly because of the address. It needs a wallet client to get the signer address, but it needs the address to create the wallet client 😕
| import type { Centrifuge } from './Centrifuge.js' | ||
| import type { CentrifugeQueryOptions } from './types/query.js' | ||
|
|
||
| export class Entity { |
There was a problem hiding this comment.
curious why you opted to redefine functions on the class (_transact, _query, _transactSequence) instead of extending the Centrifuge class and just overriding where necessary e.g the query keys
There was a problem hiding this comment.
Extending the Centrifuge class for each entity would result in there being many instances of it. But there needs to be only one, because that's where the cache lives. the extra _transact and _transactSequence here are a small convenience so you can do, this._transact() instead of this._root._transact(). I don't think the functions themselves could be moved to Entity, because we'll need something like Centrifuge.createPool() at some point.
src/Pool.ts
Outdated
| activeDomains() { | ||
| return this._query(null, () => { | ||
| return this.domains().pipe( | ||
| switchMap((domains) => { | ||
| return combineLatest( | ||
| domains.map((domain) => | ||
| domain.isActive().pipe( | ||
| timeout(8000), | ||
| catchError(() => { | ||
| return of(false) | ||
| }) | ||
| ) | ||
| ) | ||
| ).pipe( | ||
| map((isActive) => { | ||
| return domains.filter((_, index) => isActive[index]) | ||
| }) | ||
| ) | ||
| }) | ||
| ) | ||
| }) |
There was a problem hiding this comment.
would this fit better in the PoolDomain entity?
There was a problem hiding this comment.
A PoolDomain concerns only a single chain. It has the isActive method, which returns whether the pool is active on that domain. Like Pool.domains() returns all PoolDomains for each existing chain, Pool.activeDomains() returns only the PoolDomains where the pool is active.
There was a problem hiding this comment.
still think this particular method doesn't belong here. Maybe we should rethink the PoolNetwork entity and figure out how it can handle all networks the pool is deployed on too. Possibly it's the name PoolNetwork that's just misleading in this case. Either way we should discuss how granular we want out entities to be and come up with a strategy together
There was a problem hiding this comment.
The PoolNetwork entity, as I imagine it, represents the presence of a pool on a single network. I see it being used to query if the pool is active, get the tranche token, get the liquidity pools for the pool on a specific network. As well as deploying the tranches/liquidity pools/etc. Everything that is not network specific (like the NAV, TVL, closing an epoch, creating an Asset, etc.) can live in the Pool itself. The word "network" maybe makes it sound a bit ambiguous, but it's a common term to mean a single chain. Still happy to iterate on this though, and we can definitely brainstorm some ideas for how the entities may be laid out, and how granular they should be.
src/utils/transaction.ts
Outdated
| amount: bigint | ||
| ) { | ||
| let domainOrCurrency: any = currencyAddress | ||
| if (currencyAddress.toLowerCase() === '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48') { |
There was a problem hiding this comment.
what contract is this?
There was a problem hiding this comment.
can I recommend storing it in a named variable instead?
* Use signer in setup and remove unused conditional * Add impersonation functionality to tenderlyFork by overriding request * Add test file for accounts * Add minimal readme docs * Fix failing test * Add test case for sender address
Description
Starts out the Centrifuge class with functionality for queries, using Observables, but allowing them to be awaited. Also includes caching and deduplication. A few example queries are added to get the liquidity pool domains where a pool is active.
the
_queryfunction caches the obserable by the keys given, similar toreact-query. Everytime the same keys are passed, the same observable is returned._queryalso multicasts the observable. So if there are multiple subscribers, the underlying observable is only subscribed to once, so data isn't fetched in duplicate.nullcan be passed as keys, in which case the observable isn't cached or multicasted. It can be used when a query is only transforming/aggregating the results from other queries and doesn't need to be cached.Usage: