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

transfer primitive cardano #280

Merged
merged 23 commits into from
Mar 19, 2024
Merged

transfer primitive cardano #280

merged 23 commits into from
Mar 19, 2024

Conversation

ecioppettini
Copy link
Contributor

@ecioppettini ecioppettini commented Jan 12, 2024

Currently tied to this: dcSpark/carp#173

There are 2 things on this PR.

Pagination

First of all, I reworked the presync in order to support pagination with a tx-block hash pair. This requires a modified carp pagination method where there is a (epoch_from, epoch_to] filter. Potentially the upper bound can be removed with some tweaks, since getting extra data can be solved by just buffering this data for later. But the lower bound seems to be required?

The unfortunate thing is that keeping the existing cde's working with the slot ranges was not really as trivial as I thought at the beginning, so there is more code to handle like that I liked.

There are two ways things can go from here:

  1. We merge this, then rework the carp endpoints with the other pagination system, and then remove here the code related to slot pagination cursors.
  2. We rework the carp endpoints before merging this.

EDIT: this is tackled by #307 now.

There is also the option of adding a new transaction history endpoint that uses slot ranges, but then anyway we would have to use the stuff here eventually.

Also, with the changes here even the slot ranges pagination should be a bit better during the presync, since now each cde runs on it's own, instead of running all of them in tandem (since now each one has it's own slot cursor)

Transfer CDE

Config

extensions:
   - name: "Cardano Transfer"
     type: cardano-transfer 
     credential: addr_test1wz6t5wlw4u2tzjt3u3syqf72t98gl5247rxm7xk6nj7n39ql4j96k
     startSlot: 38152234
     scheduledPrefix: ct

Parsing

Currently the outputs field is basically a json, since I'm not sure otherwise how to express it with the grammar/parsing.

The data provided in the event is currently the list of input addresses, so that the game can know who transfered, the outputs with amounts (per asset). This probably requires filtering the outputs that don't go to the address provided, but I'm not sure. Also the metadata is just a hex.

The raw cbor tx currently it's just stored in the db. I'm thinking that maybe it would be better to get that through some util function (tx by hash) instead of getting it in the event?

'cardanoTransfer  = ct|txId|metadata|inputCredentials|outputs'

const cardanoTransfer: ParserRecord<CardanoTransfer> = {
  txId: PaimaParser.NCharsParser(0, 64),
  metadata: PaimaParser.OptionalParser(null, PaimaParser.RegexParser(/[a-f0-9]*/)),
  inputCredentials: PaimaParser.ArrayParser({
    item: PaimaParser.RegexParser(/[a-f0-9]*/),
  }),
  outputs: (keyName: string, input: string) => {
    return JSON.parse(input);
  },
};

export interface CardanoTransfer {
  txId: string;
  metadata: string | null;
  inputCredentials: string[];
  outputs: { asset: { policyId: string; assetName: string }; amount: string; address: string }[];
}

@ecioppettini ecioppettini force-pushed the enzo/transfer-cde-cardano branch 3 times, most recently from addf8e6 to f02927d Compare March 4, 2024 18:47
);

CREATE TABLE cde_cardano_transfer (
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 actually starting to wonder if it does actually make sense to store this stuff... There are currently no utility function using this, and currently I'm not sure why would you want to do something with this.

Copy link
Contributor

Choose a reason for hiding this comment

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

As in you want to pass this info to the state machine without actually storing it to longer term storage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the question can also be reversed into: "What util functions do we need here?". Would a game ever want to fetch the transaction by id? For example.

Currently they may need to because the actual rax tx is not in the concise repr, but we can also add the rawTx there. But you could also just go to the db directly.

Or could you want to get all the transactions for a certain address? Or the latest? Because in this case we should also have a table indexing the txs by input address, for example.

@SebastienGllmt
Copy link
Contributor

As a reminder, we'll want to add this to the docs as well

@ecioppettini
Copy link
Contributor Author

I already started adding the docs for this PaimaStudios/paima-engine-docs#32

I just wanted to check if the format makes sense, and if we need any utility functions before polishing that up.

packages/engine/paima-funnel/package.json Outdated Show resolved Hide resolved
packages/engine/paima-funnel/package.json Outdated Show resolved Hide resolved
while (true) {
const event = await timeout(
query(url, Routes.transactionHistory, {
// TODO: maybe it should be Output
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO?

packages/engine/paima-funnel/src/cde/cardanoTransfer.ts Outdated Show resolved Hide resolved
startSlot: number;
}): { from: number; to: number } => {
const cursors = this.cache.getState().cursors;
const from: number = (cursors && (cursors[cdeId] as { slot: number }).slot) || startSlot;
Copy link
Contributor

Choose a reason for hiding this comment

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

fyi there is a Javascript feature ?? that we should be using for cases like this. || should only be used instead of ?? if you care about cases other than null/undefined

Comment on lines 398 to 399
// handle the cde's that are still on slot range based 'pagination' (not
// really pagination, but emulated)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need this given we're merging all the Carp PRs?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh nvm I just noticed it's deleted by the follow-up PR

Copy link
Contributor Author

@ecioppettini ecioppettini Mar 8, 2024

Choose a reason for hiding this comment

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

we can merge that PR into this one if it makes it easier to review, though. It would also make easier to update the carp client version once released, since that one will contain the changes for all of this. Because if we make a release of carp client version now with the latest commit, we can't put it here since it won't compile because of the api changes. But making an intermediate release without the pagination for the other endpoints doesn't make that much sense.

presyncBlockHeight = maybePresyncBlockHeight;
} else {
break;
}
}

for (const network of Object.keys(networks)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to this PR, but I just noticed the loopIfStopBlockReached doesn't work anymore because it's based on STOP_BLOCKHEIGHT (single global env) whereas this function checks the env value against the block height for all networks which doesn't really make sense. We should probably open an issue to fix this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I haven't really used this feature at all, so I didn't notice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Created an issue: #324

);

CREATE TABLE cde_cardano_transfer (
Copy link
Contributor

Choose a reason for hiding this comment

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

As in you want to pass this info to the state machine without actually storing it to longer term storage?

@ecioppettini ecioppettini force-pushed the enzo/transfer-cde-cardano branch from cf534a3 to ded1a89 Compare March 19, 2024 05:01
@SebastienGllmt SebastienGllmt merged commit f2f9e03 into master Mar 19, 2024
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