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

Docs on simulate flag for AnteHandler #18907

Closed
rootulp opened this issue Dec 27, 2023 · 3 comments · Fixed by #18913
Closed

Docs on simulate flag for AnteHandler #18907

rootulp opened this issue Dec 27, 2023 · 3 comments · Fixed by #18913
Assignees
Labels
T:Docs Changes and features related to documentation.

Comments

@rootulp
Copy link
Contributor

rootulp commented Dec 27, 2023

Context

// AnteHandler authenticates transactions, before their internal messages are handled.
// If newCtx.IsZero(), ctx is used instead.
type AnteHandler func(ctx Context, tx Tx, simulate bool) (newCtx Context, err error)

Problem

No documentation exists for the simulate flag.

Proposal

Document what is the purpose of the simulate flag. Since the AnteHandler type is implemented by both antehandlers and posthandlers, it would be helpful if the docs explained how antehandlers (or posthandlers) should treat the simulate flag. For example: if simulate=true should a posthandler not make any modifications to state?

@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Dec 27, 2023
@rootulp
Copy link
Contributor Author

rootulp commented Dec 27, 2023

// If newCtx.IsZero(), ctx is used instead.

On a related note, this line seems confusing. Where is ctx used instead of newCtx? Is this applicable to a chained AnteHandler? Why not enforce that AnteHandlers propagate the ctx from one to the next?

@alexanderbez
Copy link
Contributor

@rootulp where do you suggest exactly we put this documentation? A godoc on the AnteHandler type?

@rootulp
Copy link
Contributor Author

rootulp commented Dec 28, 2023

Yep exactly.

@tac0turtle tac0turtle added T:Docs Changes and features related to documentation. and removed needs-triage Issue that needs to be triaged labels Dec 28, 2023
@alexanderbez alexanderbez self-assigned this Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:Docs Changes and features related to documentation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants