10387 simple transaction feed for Fast USDC #10452
Conversation
Deploying agoric-sdk with
|
| Latest commit: |
1de9a54
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bdc96dde.agoric-sdk.pages.dev |
| Branch Preview URL: | https://10387-simple-feed.agoric-sdk.pages.dev |
| * @param {CctpTxEvidence} evidence | ||
| */ | ||
| makeTestPushInvitation(evidence) { | ||
| return zcf.makeInvitation( |
There was a problem hiding this comment.
Maybe we can try without an actual invitation, just a method call?
There was a problem hiding this comment.
That's how Patrick's PR already works. The point of this PR is so the CLI can push evidence. That can only be done through an offer through the smart wallet
There was a problem hiding this comment.
I was thinking more about the workaround we discussed where the smart wallet would invoke the invitation maker, but the invitation maker itself would just throw without returning an invitation. To avoid the perf overhead of zoe. We'd still want this exo for managing oracle members and aggregating submissions
There was a problem hiding this comment.
I was thinking more about the workaround we discussed where the smart wallet would invoke the invitation maker,
I see. I'll give that a try.
turadg
force-pushed
the
10387-simple-feed
branch
2 times, most recently
from
2727135
to
c5b5932
Compare
| return zone.exoClassKit('Fast USDC Feed', undefined, () => ({}), { | ||
| admin: { | ||
| /** @param {CctpTxEvidence } evidence */ | ||
| addEvidence: evidence => { |
There was a problem hiding this comment.
Is submitEvidence or reportEvidence more fitting? Or maybe adopt the push verb from pushPrice and call this pushEvidence?
There was a problem hiding this comment.
"submit" sgtm, like submit for evaluation. push was ambiguous about how it would be aggregated.
| /** @type {PublishKit<CctpTxEvidence>} */ | ||
| const { publisher, subscriber } = makePublishKit(); | ||
|
|
||
| return zone.exoClassKit('Fast USDC Feed', undefined, () => ({}), { |
There was a problem hiding this comment.
consider a guard that uses CctpTxEvidenceShape
| /** @param {CctpTxEvidence } evidence */ | ||
| addEvidence: evidence => { | ||
| trace('TEMPORARY: Add evidence:', evidence); | ||
| // TODO decentralize |
There was a problem hiding this comment.
Consider:
- basic validation checks for things that can't be verified by the guard, like address parameter
- and/or, a TODO explaining what validation checks will be performed
| void observeIteration(subscribeEach(feedKit.public.getEvidenceStream()), { | ||
| updateState(evidence) { | ||
| try { | ||
| advancer.handleTransactionEvent(evidence); | ||
| } catch (err) { | ||
| trace('🚨 Error handling transaction event', err); | ||
| } | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Nice, first time seeing this API. Surprised the second argument doesn't need to be a remotable, but that's convenient here.
Are there scenarios where we'd hid the fail or finish handlers and need to resubscribe?
There was a problem hiding this comment.
Good questions. It may need to be durable but that's for Milestone 2.
If the stream dies, I don't think resubscribing would be safe. It's an unhandled rejection that will need a Core Eval to repair.
| * @param {CctpTxEvidence} evidence | ||
| */ | ||
| async makeTestPushInvitation(evidence) { | ||
| await feedKit.admin.addEvidence(evidence); |
There was a problem hiding this comment.
Recognize this is a test method, but why change state in the invitationMaker instead of the offerHandler? If addEvidence were to throw, will smart-wallet get the error message?
There was a problem hiding this comment.
why change state in the invitationMaker instead of the offerHandler
If addEvidence were to throw, will smart-wallet get the error message?
Good question. The throw would happen on this line so it would be an IMMEDIATE_OFFER_ERROR and show up as an error in the wallet status.
I added a todo to test that when we integrate with smart-wallet in bootstrap tests
There was a problem hiding this comment.
Ok cool, I see now.
I added a todo to test that when we integrate with smart-wallet in bootstrap tests
🙏
| setJig: jig => { | ||
| jig.chainHub.registerChain('osmosis', fetchedChainInfo.osmosis); | ||
| }, |
There was a problem hiding this comment.
Nice, great approach for now. We'll need to revisit in a future milestone when we do bootstrap and multichain testing.
| return zcf.makeInvitation(async cSeat => { | ||
| trace('Offer made on noop invitation'); | ||
| cSeat.exit(); | ||
| return 'noop; evidence was pushed in the invitation maker call'; |
There was a problem hiding this comment.
Consider using undefined, as we've adopted this as the return value for many offers. This might help keep muscle-memory fresh for recognizing result: undefined as a happy path offer result.
There was a problem hiding this comment.
as the return value for many offers
That makes sense for offers that get used, but this one is not supposed to be used. So it should be conspicuously different.
turadg
force-pushed
the
10387-simple-feed
branch
from
1de9a54
to
8eb7dee
Compare
| */ | ||
|
|
||
| const trace = makeTracer('TxFeed', true); | ||
|
|
||
| export const INVITATION_MAKERS_DESC = 'transaction oracle invitation'; |
There was a problem hiding this comment.
consider a more global name (wrt fusdc) since we're exporting
closes: #10387
Description
A basic transaction feed component. It has a public invitation maker that isn't for production but will work for contract tests as Advancer and Settle are built.
Security Considerations
Scaling Considerations
Documentation Considerations
Testing Considerations
Upgrade Considerations