-
Notifications
You must be signed in to change notification settings - Fork 4
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
feature: Add initial support for runtime rofl transactions #812
Conversation
e57976e
to
fa87391
Compare
fa87391
to
5ccc938
Compare
5ccc938
to
df2f5c0
Compare
df2f5c0
to
97f8baa
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.
Nice, the Explorer folks might be interested in designing iconography for rofl transactions. cc @csillag
case "rofl.Create": | ||
if handler.RoflCreate != nil { | ||
var body rofl.Create | ||
if err := cbor.Unmarshal(call.Body, &body); err != nil { | ||
return fmt.Errorf("unmarshal rofl create: %w", err) | ||
} | ||
if err := handler.RoflCreate(&body); err != nil { | ||
return fmt.Errorf("rofl create: %w", err) | ||
} | ||
} | ||
case "rofl.Update": | ||
if handler.RoflUpdate != nil { | ||
var body rofl.Update | ||
if err := cbor.Unmarshal(call.Body, &body); err != nil { | ||
return fmt.Errorf("unmarshal rofl update: %w", err) | ||
} | ||
if err := handler.RoflUpdate(&body); err != nil { | ||
return fmt.Errorf("rofl update: %w", err) | ||
} | ||
} |
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.
Is there any relevant information in the CallResult for rofl transactions? Or is it left for a future ticket?
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.
"rofl.Create" has App ID, others are empty. But we don't have anything to do with it at the moment, will leave it as a followup #807
@@ -0,0 +1,9 @@ | |||
feature: Add support for runtime rofl transactions | |||
|
|||
New runtime transactions: |
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.
Not an ask for this PR, but we should think about adding another e2e regression suite that encompasses rofl/sapphire/consensusaccount txs and the rest of the new features being added :)
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.
Good idea, opened #826
Already in progress oasisprotocol/explorer#1641 |
97f8baa
to
4684659
Compare
Fixes: #806
There were already some ROFL apps deployed on testnet for sapphire, so until we reindex, we will miss those transactions/events for now.
It is also possible to write a script that goes over runtime transactions and events for Sapphire and backfills the rofl events and transactions if we won't do a reindex and will still want the missed data - we can do this at a a later time.
For forntend, just displaying the transations.body and formating the events nicely will probably be good for first version.