-
Notifications
You must be signed in to change notification settings - Fork 63
fix(cardano-services): correct mapping of chain history redeemer purpose #1176
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
fix(cardano-services): correct mapping of chain history redeemer purpose #1176
Conversation
packages/cardano-services/src/ChainHistory/DbSyncChainHistory/mappers.ts
Outdated
Show resolved
Hide resolved
|
packages/cardano-services/src/ChainHistory/DbSyncChainHistory/mappers.ts
Outdated
Show resolved
Hide resolved
a791083
to
20e8d5c
Compare
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.
Great work, @mkazlauskas 🚀
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!
20e8d5c
to
5662edc
Compare
packages/cardano-services/src/ChainHistory/DbSyncChainHistory/mappers.ts
Outdated
Show resolved
Hide resolved
bded200
8f5cc2d
to
c3c9c41
Compare
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.
Very nice!
packages/cardano-services/src/ChainHistory/DbSyncChainHistory/mappers.ts
Show resolved
Hide resolved
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.
Fantastic work, @mkazlauskas
@@ -95,7 +95,7 @@ export interface WithdrawalModel { | |||
|
|||
export interface RedeemerModel { | |||
index: number; | |||
purpose: string; | |||
purpose: 'cert' | 'mint' | 'spend' | 'reward' | 'voting' | 'proposing'; |
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.
👌
Probably it is possible to automatically get them from RedeemerModel
but this is way better than string
Maybe keysof RedeemerModel["purpose"]
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.
Sorry @gytis-ivaskevicius but I don't understand what you're suggesting as the source, this is the RedeemerModel
🤔
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.
@gytis-ivaskevicius next action on you here
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.
Realized that what I wrote was invalid, idea was to take purpose type from elsewhere to avoid hardcoding. Not sure if its possible but anyways. No action on this
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 see, that would be difficult to achieve, it's options from db-sync schema
…e and propose based on latest conway.cddl
based on db-sync schema doc
c3c9c41
to
b27bcac
Compare
2a1562c
Context
This fails:
with
Proposed Solution
Correctly map redeemer purpose based on values documented in db-sync schema
Important Changes Introduced