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

Refresh events CAP #1371

Merged
merged 7 commits into from
Aug 10, 2023
Merged

Refresh events CAP #1371

merged 7 commits into from
Aug 10, 2023

Conversation

sisuresh
Copy link
Contributor

No description provided.

that the user can fill out with smaller values so downstream systems can provide
more narrow filtering and subscription options without prior knowledge of the
`data` format. There can only be four topics, and if the type is `Bytes`, cannot
be more than 64 bytes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This 64 byte limit doesn't make much sense and should probably be removed. You can bypass it by using a Vec or String.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's probably worthwhile specifying the event limits more clearly.

  • Should the topics size be network configurable? 4 seems a bit arbitrary and too restrictive.
  • Should there be a size limit in both the topics and data? Unbounded events means contracts can submit a single event up to 500k (meta data size limit) repeatedly, causing downstream stress in processing them.

@sisuresh sisuresh requested a review from dmkozh August 10, 2023 17:12
core/cap-0046-08.md Outdated Show resolved Hide resolved
core/cap-0046-08.md Outdated Show resolved Hide resolved
core/cap-0046-08.md Outdated Show resolved Hide resolved
@sisuresh sisuresh enabled auto-merge (squash) August 10, 2023 19:27
core/cap-0046-08.md Outdated Show resolved Hide resolved
@sisuresh sisuresh merged commit 1009951 into stellar:master Aug 10, 2023
2 checks passed
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.

3 participants