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

fix: fix overriding tx index of duplicated txs #498

Merged
merged 3 commits into from
Nov 15, 2022

Conversation

ulbqb
Copy link
Member

@ulbqb ulbqb commented Nov 14, 2022

Description

The function OnStart() starts a goroutine to handle block events and transactions continuously. However, there is no duplicate check against batch before performing an invocation on txIdxr.AddBatch(). Moreover, a new transaction containing failed results may override a successful transaction that has already been indexed in the db, which may lead to confusion in accounting calculations.

This PR addresses the above issue. And the content is the same as here.

@codecov
Copy link

codecov bot commented Nov 14, 2022

Codecov Report

Merging #498 (4941250) into main (0681636) will increase coverage by 0.04%.
The diff coverage is 80.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #498      +/-   ##
==========================================
+ Coverage   65.44%   65.48%   +0.04%     
==========================================
  Files         278      278              
  Lines       37936    37966      +30     
==========================================
+ Hits        24828    24863      +35     
+ Misses      11305    11297       -8     
- Partials     1803     1806       +3     
Impacted Files Coverage Δ
state/txindex/indexer_service.go 69.04% <80.00%> (+6.08%) ⬆️
types/block.go 72.78% <0.00%> (-0.78%) ⬇️
consensus/reactor.go 74.49% <0.00%> (-0.71%) ⬇️
p2p/conn/connection.go 79.80% <0.00%> (-0.58%) ⬇️
consensus/state.go 73.67% <0.00%> (-0.18%) ⬇️
proxy/multi_app_conn.go 47.66% <0.00%> (ø)
light/client.go 62.05% <0.00%> (+0.44%) ⬆️
blockchain/v0/pool.go 79.58% <0.00%> (+0.77%) ⬆️
abci/client/socket_client.go 72.96% <0.00%> (+0.87%) ⬆️
p2p/pex/pex_reactor.go 80.28% <0.00%> (+1.23%) ⬆️
... and 2 more

@ulbqb ulbqb marked this pull request as ready for review November 14, 2022 09:23
CHANGELOG.md Outdated Show resolved Hide resolved
@ulbqb ulbqb changed the title Fix overriding tx index of duplicated txs fix: fix overriding tx index of duplicated txs Nov 14, 2022
@tnasu tnasu added the C: bug Classification: Something isn't working label Nov 14, 2022
This reverts commit e02f6b8.
@torao
Copy link
Contributor

torao commented Nov 15, 2022

What's the reason to incorporate only this tendermint/tendermint#8625 PR now?

@ulbqb
Copy link
Member Author

ulbqb commented Nov 15, 2022

@torao
I added the issue this PR addresses to Description. I think you can see why.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: bug Classification: Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants