Skip to content

Conversation

@teogeb
Copy link
Contributor

@teogeb teogeb commented May 23, 2025

Added stream ID length check to all events handlers which process stream related events. If the stream ID is more than 1000 characters, the event is just ignored. That causes the stream to be fully ignored in The Graph (it won't appear e.g. in the streams query).

TODO (in a separate PR)

We need to document to our public document that overlong streams are not supported (i.e. the behavior is undefined in our system for those streams).

Manual test

PRIVATE_KEY="1111111111111111111111111111111111111111111111111111111111111111"
NOW=$(date +%Y%m%d%H%M%S)
LONG_SNIPPET=$(printf 'a%.0s' {1..2000})
echo 'Create streams'
npx tsx bin/streamr.ts internal token-mint self 1000000 1000000 --env dev2 --private-key $PRIVATE_KEY
npx tsx bin/streamr.ts stream create /short1-$NOW --env dev2 --private-key $PRIVATE_KEY
npx tsx bin/streamr.ts stream create /long-$NOW-$LONG_SNIPPET --env dev2 --private-key $PRIVATE_KEY
npx tsx bin/streamr.ts stream create /short2-$NOW --env dev2 --private-key $PRIVATE_KEY
echo 'Wait for The Graph to index'
sleep 10s
echo 'Query streams:'
npx tsx bin/streamr.ts stream search $NOW --env dev2

Related changed

  • Simplified getPermissionId() as the slicing and concatenation which is no longer needed

Future improvements

  • We could avoid code repetition by using some kind of wrapper function which does the length check

@linear
Copy link

linear bot commented May 23, 2025

@teogeb teogeb requested review from harbu and jtakalai May 23, 2025 12:17
Copy link

@jtakalai jtakalai left a comment

Choose a reason for hiding this comment

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

I'll leave a differing opinion here for the record. I don't think this restriction is necessary or useful. Introducing undefined behaviour is unnecessary, so is introducing discrepancy between smart contract state and subgraph state. This has not been done before, and the complexity of that implication is hard to foresee.

These changes have been extensively discussed, however, and since this PR does legitimately fix the original problem of breaking the subgraph, I'm not going to request changes.

@teogeb teogeb merged commit 454e428 into master May 23, 2025
3 checks passed
@teogeb teogeb deleted the overlong-stream-ids branch May 23, 2025 13:32
teogeb added a commit that referenced this pull request Jun 16, 2025
)

Updated the comment. It mentioned slicing which we removed in
#993.
rollup-ravenx5hy8 added a commit to rollup-ravenx5hy8/network-contracts that referenced this pull request Nov 6, 2025
…021)

Updated the comment. It mentioned slicing which we removed in
streamr-dev/network-contracts#993.
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