lazy decode messages #17
Conversation
|
It would be good to do the same for decoding events and transactions where possible and have some tests that show that the lazy loading works where possible. We would have to use jest spy features. |
| @@ -169,7 +192,9 @@ function wrapCosmosMsg( | |||
| block: block, | |||
| msg: { | |||
| typeUrl: rawMessage.typeUrl, | |||
| ...api.decodeMsg<any>(rawMessage), | |||
| get decodedMsg() { | |||
There was a problem hiding this comment.
Is this changing the CosmosMessage type? Will this require updating other projects?
There was a problem hiding this comment.
Looks like that the data were originally accessed through msg property, will have to be accessed through msg.decodedMsg
There was a problem hiding this comment.
We could look into a way of transforming it so that the projects or dictionary remains unaffected
packages/node/src/utils/cosmos.ts
Outdated
| evt.msg.msg = { | ||
| ...evt.msg.msg, | ||
| ...evt.msg.msg.decodedMsg, | ||
| }; | ||
| delete evt.msg.msg.decodedMsg; |
There was a problem hiding this comment.
Is this hoisting the change from the wrappers? I think this is quite hard to follow.
Would we be better off making a breaking change to get more usable types?
packages/node/src/utils/cosmos.ts
Outdated
| .filter((message) => { | ||
| filters.find((filter) => filterMessageData(message, filter)); | ||
| }) | ||
| .map((msg) => { |
There was a problem hiding this comment.
Would this need to be done if there are no filters too?
There was a problem hiding this comment.
I just realized that this function will be only be called for custom datasources. It's a seperate mechanism for runtime datasources:
There was a problem hiding this comment.
And it wouldn't be done if there are no filters but, it has to be done.
There was a problem hiding this comment.
And this transformation doesn't happen for runtime datasources. msg.decodedMsg would still exist.
packages/types/src/interfaces.ts
Outdated
| msg: any; | ||
| msg: { | ||
| typeUrl: string; | ||
| decodedMsg: any; |
There was a problem hiding this comment.
Can we make this generic? CosmosMessage<T = any> and decodedMsg: T
stwiname
force-pushed
the
http-keep-alive
branch
from
bc2eac5 to
d661e41
Compare
|
@DeveloperInProgress I think this is looking good but I think wrapping the data types will cause everything to be loaded. Problem is here: https://github.com/subquery/subql-cosmos/blob/main/packages/node/src/utils/cosmos.ts#L245-L255 I think we might need to lazy load the events/messages/transactions, that way if there's no event handlers then we don't wrap them. Can we have a test that calls |
No description provided.