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

[WIP] common,bp,txapp: call notices and row results in log, and use log from tx routes #1368

Closed
wants to merge 1 commit into from

Conversation

jchappelow
Copy link
Member

JUST TESTING.

I'm experimenting with putting results from action execution in the transaction log:

  • notices
  • scanned rows from an action return, either a single value, single record/row, or a table

I think there are gotchas when a table is returned, but I like the possibility to have action returns go somewhere.

@brennanjl
Copy link
Collaborator

I think there are gotchas when a table is returned, but I like the possibility to have action returns go somewhere.

Curious, what might these be?

@jchappelow
Copy link
Member Author

I think there are gotchas when a table is returned, but I like the possibility to have action returns go somewhere.

Curious, what might these be?

I need to think it though, but it seems like if this is the case then it adds some considerations to the action author. For instance, perhaps an action is best suited to serve other actions, so it returns largish tables, and the other actions return more reasonably (small) results. So directly calling the action with a large table return ends up with a transaction with a large log. Maybe modifiers can avoid that, or perhaps log length limits?

Comment on lines -319 to -322
// TODO: once we are able to store execution logs in the block store, we should propagate the discarded
// return value here.
_, err := app.Engine.Call(makeEngineCtx(ctx), app.DB, d.namespace, d.action, d.args[i], func(r *common.Row) error {
// we throw away all results for execute actions
Copy link
Member Author

Choose a reason for hiding this comment

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

@charithabandi Since you were asking, this is the very old TODO I was attempting to address. There are two things we can put in the tx log: (1) the db tx "logs" which are emitted notices, and (2) returned data from the action.

My hesitation with this PR is twofold: (1) if the schema author is not cautious about the size of data returned and possibly modifiers (like owner or private), this could result in enormous txResults that bloat the chain and network traffic in commit info, and (2) what is the format and structure for the tx log (I'm just experimenting with a human readable format here).

@jchappelow
Copy link
Member Author

Closing in favor of #1439

@jchappelow jchappelow closed this Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants