refactor(baseapp): handle cases for PostHandler in runTx #14593
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if app.postHandler != nil { | ||
| // Run optional postHandlers with a context branched off the ante handler ctx | ||
| postCtx, postCache := app.cacheTxContext(ctx, txBytes) |
Check warning
Code scanning / CodeQL
Useless assignment to local variable
Vvaradinov
requested review from
alexanderbez
and removed request for
JeancarloBarrios and
likhita-809
|
|
||
| newCtx, err := app.postHandler(postCtx, tx, mode == runTxModeSimulate, err == nil) | ||
| if err != nil { | ||
| return gInfo, nil, anteEvents, priority, err | ||
| // return result in case the pointer has been modified by the post decorators |
There was a problem hiding this comment.
what pointer? There's no pointer being passed into postHandler.
| if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) { | ||
| // append the events in the order of occurrence | ||
| postEvents = newCtx.EventManager().ABCIEvents() | ||
| events = make([]abci.Event, len(anteEvents)+len(postEvents)) | ||
| copy(events[:len(anteEvents)], anteEvents) | ||
| copy(events[len(anteEvents):], postEvents) | ||
| result.Events = append(result.Events, events...) | ||
| } else { | ||
| events = make([]abci.Event, len(postEvents)) | ||
| copy(events, postEvents) | ||
| result.Events = append(result.Events, postEvents...) | ||
| } |
There was a problem hiding this comment.
This is really messy IMO. runTx is arguably the most important method of BaseApp and the state machine in general and it's already not written in the most friendly and intuitive way. These changes exacerbate that IMO.
Also the change from anteEvents to events is invalid. It should remain anteEvents because that's what we're returning.
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
what is the status here? @JeancarloBarrios @alexanderbez |
|
I'm not thrilled with the changes, specifically all the event logic. So we're gonna sync tomorrow. |
Description
This is a follow up PR to #13940. It includes the 3 cases for
PostHandlerlogic inrunTx.Closes: #14592 #13942
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change