Skip to content

ABCI: Fix ReCheckTx for Socket Client#6124

Merged
mergify[bot] merged 4 commits intomasterfrom
bez/5439-fix-didRecvResponse
Feb 17, 2021
Merged

ABCI: Fix ReCheckTx for Socket Client#6124
mergify[bot] merged 4 commits intomasterfrom
bez/5439-fix-didRecvResponse

Conversation

@alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Feb 17, 2021 •
edited
Loading

Fixes the race condition between a callback being set and called during ReCheckTx. Note, I do not see equivalent logic in the gRPC client (anymore) as #5439 suggests, so only the socket client was updated.

closes: #5439

@alexanderbez alexanderbez changed the title Fix ReCheckTx for Socket Client ABCI: Fix ReCheckTx for Socket Client Feb 17, 2021
@alexanderbez alexanderbez marked this pull request as ready for review February 17, 2021 15:35
@alexanderbez alexanderbez self-assigned this Feb 17, 2021
@alexanderbez alexanderbez added C:abci Component: Application Blockchain Interface S:automerge Automatically merge PR when requirements pass labels Feb 17, 2021
@codecov
Copy link

codecov bot commented Feb 17, 2021 •
edited
Loading

Codecov Report

Merging #6124 (01c64e8) into master (b6be889) will increase coverage by 0.13%.
The diff coverage is 17.64%.

@@            Coverage Diff             @@
##           master    #6124      +/-   ##
==========================================
+ Coverage   60.65%   60.78%   +0.13%     
==========================================
  Files         276      276              
  Lines       25646    25649       +3     
==========================================
+ Hits        15555    15592      +37     
+ Misses       8469     8437      -32     
+ Partials     1622     1620       -2
Impacted Files Coverage Δ
abci/client/client.go 32.35% <12.50%> (-7.65%) ⬇️
abci/client/socket_client.go 38.54% <100.00%> (-0.62%) ⬇️
crypto/sr25519/pubkey.go 43.47% <0.00%> (-8.70%) ⬇️
privval/signer_server.go 89.47% <0.00%> (-5.27%) ⬇️
mempool/reactor.go 67.14% <0.00%> (-5.00%) ⬇️
p2p/peer.go 48.76% <0.00%> (-1.66%) ⬇️
mempool/clist_mempool.go 80.21% <0.00%> (-0.72%) ⬇️
p2p/switch.go 59.90% <0.00%> (-0.47%) ⬇️
proxy/multi_app_conn.go 48.05% <0.00%> (ø)
... and 13 more

@alexanderbez alexanderbez added the S:backport-to-v0.34.x Tell mergify to backport the PR to v0.34.x label Feb 17, 2021
@alexanderbez alexanderbez marked this pull request as draft February 17, 2021 15:48
@alexanderbez alexanderbez marked this pull request as ready for review February 17, 2021 16:09
tessr
tessr reviewed Feb 17, 2021
View reviewed changes
tac0turtle
tac0turtle approved these changes Feb 17, 2021
View reviewed changes
Copy link
Contributor

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@alexanderbez @tessr
Update CHANGELOG_PENDING.md
01c64e8 
Co-authored-by: Tess Rinearson <tess.rinearson@gmail.com>
@mergify mergify bot merged commit 5b52f87 into master Feb 17, 2021
@mergify mergify bot deleted the bez/5439-fix-didRecvResponse branch February 17, 2021 17:03
mergify bot pushed a commit that referenced this pull request Feb 17, 2021
@alexanderbez
ABCI: Fix ReCheckTx for Socket Client (#6124)
ad16c37 
Fixes the race condition between a callback being set and called during ReCheckTx. Note, I do not see equivalent logic in the gRPC client (anymore) as #5439 suggests, so only the socket client was updated.

closes: #5439

(cherry picked from commit 5b52f87)

# Conflicts:
#	CHANGELOG_PENDING.md
@mergify mergify bot mentioned this pull request Feb 17, 2021
Merged
alexanderbez pushed a commit that referenced this pull request Feb 18, 2021
@mergify
abci: Fix ReCheckTx for Socket Client (bp #6124) (#6125)
8d3c36c
@alexanderbez alexanderbez mentioned this pull request May 7, 2021
Closed
4 tasks
@cmwaters cmwaters mentioned this pull request Jul 5, 2021
Merged
@cmwaters cmwaters mentioned this pull request Feb 22, 2022
Merged
@tnasu tnasu mentioned this pull request Apr 6, 2022
Closed
This was referenced Apr 22, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ebuchman ebuchman Awaiting requested review from ebuchman

@cmwaters cmwaters Awaiting requested review from cmwaters

3 more reviewers

@tessr tessr tessr approved these changes

@melekes melekes melekes approved these changes

@tac0turtle tac0turtle tac0turtle approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alexanderbez alexanderbez

Labels

C:abci Component: Application Blockchain Interface S:automerge Automatically merge PR when requirements pass S:backport-to-v0.34.x Tell mergify to backport the PR to v0.34.x

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

abci/grpc: panic during concurrent CheckTx

4 participants

@alexanderbez @tessr @melekes @tac0turtle