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

feat: remove concurrent transactions and messages handling #86

Merged
merged 2 commits into from
Dec 2, 2022

Conversation

RiccardoM
Copy link
Contributor

@RiccardoM RiccardoM commented Nov 29, 2022

Description

This PR removes the concurrent handling of transactions and messages. This is because it might cause some wrongful handling then a transaction contains messages that perform opposite operations.

As an example, consider a transaction that contains, in the given order:

  • one MsgRevokeFeeAllowance from Alice to Bob
  • one MsgGrantFeeAllowance from Alice to Bob

If those messages are handled concurrently, since concurrency does not guarantee the order of handling, the following cases are both likely to happen:

  1. the MsgRevokeFeeAllowance gets handled before MsgGrantFeeAllowance
  2. the MsgGrantFeeAllowance gets handled before MsgRevokeFeeAllowance

If the parser is storing the existing fee allowances, then the following results are equally likely to happen:

  1. the database wil contain the proper fee allowance (if MsgRevokeFeeAllowance gets handled before MsgGrantFeeAllowance), or
  2. the database will not contain the fee allowance (if MsgGrantFeeAllowance gets handled before MsgRevokeFeeAllowance)

Obviously, the second case is not the correct one and is only due to the concurrent handling of messages.

The same might happen on a transaction level, with two transactions containing similar messages.

To solve this, we need to remove the concurrent handling of transactions and messages.

Checklist

  • Targeted PR against correct branch.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote unit tests.
  • Re-reviewed Files changed in the Github PR explorer.

@RiccardoM RiccardoM changed the title feta: remove concurrent transactions and messages handling feat: remove concurrent transactions and messages handling Nov 29, 2022
@RiccardoM RiccardoM marked this pull request as ready for review November 29, 2022 16:11
@RiccardoM RiccardoM added the automerge Automatically merge PR once all prerequisites pass label Nov 29, 2022
@RiccardoM RiccardoM mentioned this pull request Nov 29, 2022
4 tasks
RiccardoM added a commit that referenced this pull request Nov 30, 2022
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰    
v    Before smashing the submit button please review the checkboxes.
v If a checkbox is n/a - please still include it but + a little note why
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Description
This PR prepares the code for release `v4.0.0` by:
- updating `CHANGELOG.md` to contain the proper changes
- updating the `go.mod` to use the proper go module
(`github.com/forbole/juno/v4`)

Depends-on: #84
Depends-on: #86

## Checklist
- [x] Targeted PR against correct branch.
- [ ] Linked to Github issue with discussion and accepted design OR link
to spec that describes this work.
- [ ] Wrote unit tests.  
- [x] Re-reviewed `Files changed` in the Github PR explorer.
Copy link
Contributor

@huichiaotsou huichiaotsou left a comment

Choose a reason for hiding this comment

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

LGTM

@RiccardoM RiccardoM merged commit 13b4694 into cosmos/v0.45.x Dec 2, 2022
@RiccardoM RiccardoM deleted the riccardom/revert-concurrent-handling branch December 2, 2022 20:39
MonikaCat pushed a commit that referenced this pull request Jan 19, 2024
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰    
v    Before smashing the submit button please review the checkboxes.
v If a checkbox is n/a - please still include it but + a little note why
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Description
This PR prepares the code for release `v4.0.0` by:
- updating `CHANGELOG.md` to contain the proper changes
- updating the `go.mod` to use the proper go module
(`github.com/forbole/juno/v4`)

Depends-on: #84
Depends-on: #86

## Checklist
- [x] Targeted PR against correct branch.
- [ ] Linked to Github issue with discussion and accepted design OR link
to spec that describes this work.
- [ ] Wrote unit tests.  
- [x] Re-reviewed `Files changed` in the Github PR explorer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Automatically merge PR once all prerequisites pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants