Skip to content

SSZ block V2 stack overflow#5076

Closed
realbigsean wants to merge 3 commits intosigp:unstablefrom
realbigsean:box-ssz-publish
Closed

SSZ block V2 stack overflow#5076
realbigsean wants to merge 3 commits intosigp:unstablefrom
realbigsean:box-ssz-publish

Conversation

@realbigsean
Copy link
Member

Issue Addressed

When publishing a full block via the SSZ V2 endpoint, we get a stack overflow, it seems to be caused by this: seanmonstar/warp#811

Added .boxed() has fixed it for me testing on kurtosis. I've probably added more than necessary, but being a bit conservative.

@realbigsean realbigsean added the bug Something isn't working label Jan 16, 2024
.uor(post_beacon_blinded_blocks_v2_ssz)
.boxed(),
)
.boxed()
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this is the critical one, although I feel like the handler/filter shouldn't be too massive at this point as it is basically:

post().and(header_exact.and(boxed_handler))

which would have a type something like:

And<Post, And<HeaderExact, Boxed<Filter<..>>>

The size of this object on the stack does not seem to me like it should be large. An And is only as large as its two elements added together:

https://github.com/seanmonstar/warp/blob/7b07043cee0ca24e912155db4e8f6d9ab7c049ed/src/filter/and.rs#L13-L16

The fact we've already boxed most of the handler should help here, I would think.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking it may be more related to the stack size of the futures created by the filters when they execute:

https://github.com/seanmonstar/warp/blob/7b07043cee0ca24e912155db4e8f6d9ab7c049ed/src/filter/and.rs#L38-L50

AFAICT both request and response are boxed anyway

@michaelsproul
Copy link
Member

I'll push my changes here

@paulhauner
Copy link
Member

paulhauner commented Jan 22, 2024

I've just reverted cd3dde7 at Michael's suggestion. We're going to start testing this on our infra as a possible candidate for v4.6.0.

paulhauner added a commit that referenced this pull request Jan 22, 2024
Squashed commit of the following:

commit 2653bee
Author: Paul Hauner <paul@paulhauner.com>
Date:   Mon Jan 22 13:55:31 2024 +1100

    Revert "remove a couple boxes"

    This reverts commit cd3dde7.

commit cd3dde7
Author: realbigsean <seananderson33@GMAIL.com>
Date:   Tue Jan 16 18:03:24 2024 -0500

    remove a couple boxes

commit 8617ac2
Author: realbigsean <seananderson33@GMAIL.com>
Date:   Tue Jan 16 16:37:03 2024 -0500

    box ssz publish
@paulhauner paulhauner mentioned this pull request Jan 22, 2024
2 tasks
paulhauner added a commit that referenced this pull request Jan 22, 2024
Squashed commit of the following:

commit 2653bee
Author: Paul Hauner <paul@paulhauner.com>
Date:   Mon Jan 22 13:55:31 2024 +1100

    Revert "remove a couple boxes"

    This reverts commit cd3dde7.

commit cd3dde7
Author: realbigsean <seananderson33@GMAIL.com>
Date:   Tue Jan 16 18:03:24 2024 -0500

    remove a couple boxes

commit 8617ac2
Author: realbigsean <seananderson33@GMAIL.com>
Date:   Tue Jan 16 16:37:03 2024 -0500

    box ssz publish
@paulhauner
Copy link
Member

Closing in favour of #5104

@paulhauner paulhauner closed this Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working v4.6.0 ETA Q1 2024

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants