[Processor] Simplify some logic to reduce unnecessary duplicated work. #501
Conversation
grao1991
force-pushed
the
grao_opt_processor
branch
from
c3c42da to
3cb1385
Compare
grao1991
force-pushed
the
grao_opt_processor
branch
10 times, most recently
from
02c2573 to
22a7444
Compare
grao1991
requested review from
rtso,
yuunlimm,
bowenyang007 and
larry-aptos
I don't have a good way to get a full coverage for everything. I only manually sampled some rows and didn't see any difference before and after. |
|
The general code structure looks good to me, but I'm hesitant to stamp this until we do adequate testing because it's such a big change. We can either write some integration tests (@yuunlimm or @larry-aptos do either of you have bandwidth to write these tests?) or we can deploy to devnet and manually check each resource is indexed. |
grao1991
force-pushed
the
grao_opt_processor
branch
5 times, most recently
from
6e55195 to
d1ea0a4
Compare
|
Rebased my PR on top of tests. PTAL @rtso @yuunlimm @larry-aptos |
grao1991
force-pushed
the
grao_opt_processor
branch
from
d1ea0a4 to
5de3804
Compare
| if MoveResource::get_outer_type_from_write_resource(write_resource) != Self::type_str() { | ||
| return Ok(None); | ||
| } | ||
| Ok(Some(write_resource.try_into()?)) |
There was a problem hiding this comment.
Can we log the txn_version and wsc_index in case this fails? That will help us debug errors in parsing logic
This PR is trying to move some common logic/type/constant to resources.rs, avoid duplicate/unnecessary parsing/deserialization logic when converting between generic write resource and a specific resource type.
The main motivation is to improve the performance, no other behavior change is expected.
To give an idea on the performance impact, when running simple token minting workload, the processing time of token_v2_processor is 5x less than before.
This PR only handles FA and TokenV2 resources, leave other resources like Coin and Token for future PRs.