Skip to content
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

Tracelistener: TraceProcessor bug #60

Closed
clockworkgr opened this issue Jul 2, 2021 · 4 comments · Fixed by EmerisHQ/tracelistener#105
Closed

Tracelistener: TraceProcessor bug #60

clockworkgr opened this issue Jul 2, 2021 · 4 comments · Fixed by EmerisHQ/tracelistener#105
Assignees
Labels
bug Something isn't working MVP++ sdk-related tracelistener Tracelistener-related issues

Comments

@clockworkgr
Copy link
Contributor

Currently the processor passes the data to every individual module processor that owns the current key prefix.

However, key prefixes are unique PER MODULE and not overall.

So e.g. BankProcessor and DenomTraceProcessor both own key prefix 0x02 thus both attempt to process data that might be meant for the other.

At best, this is a waste of resources and unneeded/wird log entries.

At worst , although very unlikely, depending on actual data contained) it might lead to corrupted data being written to the DB.

Individual processors should be grouped by module name and TraceListener should check the store key(module name) as well as the prefix.

@gsora gsora added bug Something isn't working tracelistener Tracelistener-related issues labels Jul 5, 2021
@gsora gsora changed the title tracelistener: TraceProcessor bug Tracelistener: TraceProcessor bug Jul 5, 2021
@sgerogia sgerogia added the MVP++ label Oct 4, 2021
@gsora gsora removed their assignment Apr 4, 2022
@sgerogia
Copy link
Contributor

Cosmos PR open: cosmos/cosmos-sdk#11646

@Pitasi
Copy link
Contributor

Pitasi commented Apr 15, 2022

Cosmos SDK PR merged. I will take this issue so I can do the required changes to Tracelistener too.

@Pitasi Pitasi self-assigned this Apr 15, 2022
@Pitasi Pitasi added tracelistener Tracelistener-related issues and removed tracelistener Tracelistener-related issues labels Apr 15, 2022
@gamarin2
Copy link
Contributor

@Pitasi The changes won't be effective until they are backported in an SDK version and chains release a full node version with it. I don't see that happening short term, so probably not worth prioritizing this one rn.

@Pitasi
Copy link
Contributor

Pitasi commented Apr 15, 2022

I understand that but it's a one line change since the TL was already prepared for this so I did it anyway 😄

EmerisHQ/tracelistener#105

DeshErBojhaa pushed a commit to EmerisHQ/tracelistener that referenced this issue Apr 22, 2022
New changes in Cosmos SDK add the store name to the traces, this correspond to the processor's modules names and we can use that information to avoid "trying all the modules".

Closes EmerisHQ/demeris-backend#60
0xraiseup added a commit to 0xraiseup/EmerisHQ that referenced this issue Jun 6, 2024
New changes in Cosmos SDK add the store name to the traces, this correspond to the processor's modules names and we can use that information to avoid "trying all the modules".

Closes EmerisHQ/demeris-backend#60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working MVP++ sdk-related tracelistener Tracelistener-related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants