Adds newly supported classic ops, tweaks route and query for history #8
Conversation
aristidesstaffieri
changed the title
| contractId: edge.node.contractId, | ||
| fnName: scValToNative(xdr.ScVal.fromXDR(edge.node.topic1, "base64")), | ||
| to: scValToNative(xdr.ScVal.fromXDR(edge.node.topic3, "base64")), | ||
| from: scValToNative(xdr.ScVal.fromXDR(edge.node.topic2, "base64")), |
There was a problem hiding this comment.
there's some repetition in the return value that can be cleaned up in a future PR:
For ex: you could:
const fnName = scValToNative(xdr.ScVal.fromXDR(edge.node.topic1, "base64"));
There was a problem hiding this comment.
Hmm, I don't see where? Each scValToNative(xdr.ScVal.fromXDR(edge.node.topic1, "base64")) is for a different event in this case.
There was a problem hiding this comment.
Oh my bad - I read this wrong. Disregard my code suggestion above :) But I do wonder if some helpers could be useful here. Maybe something more like
const transferEdgeHelper = (edge) => ({
contractId: edge.node.contractId,
fnName: scValToNative(xdr.ScVal.fromXDR(edge.node.topic1, "base64")),
to: scValToNative(xdr.ScVal.fromXDR(edge.node.topic3, "base64")),
from: scValToNative(xdr.ScVal.fromXDR(edge.node.topic2, "base64")),
});
Mostly just trying to guard against these attrs from falling out of sync because of a typo
There was a problem hiding this comment.
oh yeah 100%, I'll make that helper since they're all the same shape.
There was a problem hiding this comment.
hmm actually after looking at this again I think the differences make this trickier to abstract. Each topic from each event correspond to a different key dependent on the event. So even though we see a few lines that have similar code like scValToNative(xdr.ScVal.fromXDR(some topic, "base64")), they name of the key and topic should be mapped according to what the event itself names them, so for mint - topic 2 is the to arg but for transferTo, the to arg is topic3.
Let me know if you think differently but I think this means the helper would have to be per topic which seems like less of an abstraction.
| source_account: pubKey, | ||
| asset_issuer: edge.node.contractId, | ||
| asset_code: tokenDetails.symbol, | ||
| type: "invoke_host_function", |
There was a problem hiding this comment.
you can use the enum OperationResponseType from stellar-sdk for these types
There was a problem hiding this comment.
ah nice, thank you
There was a problem hiding this comment.
so right now I cast this as a Partial<Horizon.ServerApi.InvokeHostFunctionOperationRecord> which extends from the base OperationResponseType. I think these are more accurate because they map to the specific op were building up. Let me know if I missed what you meant here but I think the underlying type for the specific op gives us a more accurate type here.
src/service/mercury/index.ts
Outdated
| } catch (error) { | ||
| this.logger.error(error); | ||
| const _error = JSON.stringify(error); | ||
| this.logger.error(_error); |
There was a problem hiding this comment.
I see that you have an established pattern of logging the raw error and then a string version of the error. I think that makes sense when the string version has some added info to make it more human readable. In this case, would this be a little noisy? Basically just seeing the same error twice in a row?
There was a problem hiding this comment.
ah I see yeah I think this is because the router has a built in logger that logs any uncaught errors in the route itself, so yeah maybe it makes sense to not log these in the services at all, good call.
There was a problem hiding this comment.
oh wait I see now that this one is just a copy pasta, removed in ce46724
Adds new classic ops recently added to Mercury.
This adds the remaining classic side ops queries to be used for account history routes.
Also fixes a few small query bugs found during testing.
Adds fallback to Horizon and transformer.
Notes -
This completes the queries needed for data parity with Horizon/Soroban RPC.