-
Notifications
You must be signed in to change notification settings - Fork 20
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
mina funnel #343
mina funnel #343
Conversation
this way it can be re-used also for mina or any other system, since there is no fundamental difference
edb39d2
to
f62b086
Compare
@@ -19,6 +19,7 @@ | |||
"dependencies": { | |||
"assert-never": "^1.2.1", | |||
"@dcspark/carp-client": "^3.1.0", | |||
"@dcspark/cardano-multiplatform-lib-nodejs": "5.2.0" | |||
"@dcspark/cardano-multiplatform-lib-nodejs": "5.2.0", | |||
"postgres": "^3.3.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we use the pg
/ pg-tx
libraries we already use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest I just didn't think about it. I started with this dep because it was being used in the archive node api, and since I took that as the base it was simpler to get started. Then I was considering porting to pgtyped eventually, but I didn't think about that. Switching to pg should be simple, I'll do that first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I think switching to pgtyped
is a lower priority. If we can at least use pg
to avoid having to depend on the postgres
package as well I think that's good enough
height: number; | ||
timestamp: string; | ||
}; | ||
// TODO: could each data by just a tuple? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO
@@ -57,7 +62,15 @@ export interface CardanoPresyncChainData { | |||
extensionDatums: ChainDataExtensionDatum[]; | |||
} | |||
|
|||
export type PresyncChainData = EvmPresyncChainData | CardanoPresyncChainData; | |||
// TODO: potentially this and Cardano can be collapsed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually want to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. They are basically isomorphic and have the same behavior. But I prefer keeping them separated.
while (true) { | ||
console.log('making query'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably you didn't mean to commit these console statements
@@ -80,6 +80,8 @@ export async function getMultipleBlockData( | |||
): Promise<ChainData[]> { | |||
const batch = new web3.BatchRequest(); | |||
|
|||
// console.log('getting multiple block data', fromBlock, toBlock); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you want to leave this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo the console.log statements
This adds a new funnel for Mina.
See PaimaStudios/paima-engine-docs#37 for docs.
TODO