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

fix(cosmos): prevent Golang error wrapping stack frame divergence #7930

Merged
merged 4 commits into from
Jun 21, 2023

Conversation

michaelfig
Copy link
Member

fixes: #7929

Description

The %w format specifier for wrapping a Golang error object propagates some stack frame information, and thereby affects consensus if sent into SwingSet (since XS heap snapshots and vat transcripts will contain these strings). Use the %s format specifier to stringify Golang error objects so SwingSet is not sensitive to them.

Security Considerations

Improves the ability to release soft-patches that change Golang stack information without affecting consensus data.

Scaling Considerations

n/a

Documentation Considerations

n/a

Testing Considerations

Some simple unit tests were written, but more comprehensive audit or testing would be useful.

@michaelfig michaelfig self-assigned this Jun 15, 2023
@michaelfig michaelfig requested a review from JimLarson June 15, 2023 15:52
@michaelfig michaelfig marked this pull request as ready for review June 15, 2023 15:52
Copy link
Contributor

@JimLarson JimLarson left a comment

Choose a reason for hiding this comment

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

I see no alternative but eternal vigilance for keeping the errors stack-free.

I also see %w in x/swingset/abci.go. It's in a panic(), but there's no local way to tell if the panic is caught. Please fix that one too.

There's also %w in daemon/cmd/genaccounts.go. This is probably purely client-side - however, there's a benefit for having the whole codebase %w-free. Consider changing those too.

@michaelfig
Copy link
Member Author

I see no alternative but eternal vigilance for keeping the errors stack-free.

Eternal vigilance... hey, I can automate that (or at least some of it)!

there's a benefit for having the whole codebase %w-free.

Will do!

@michaelfig michaelfig force-pushed the mfig-bridge-no-error-stack branch from 2879ec3 to eaffc1b Compare June 21, 2023 00:12
@michaelfig michaelfig enabled auto-merge June 21, 2023 00:13
@michaelfig michaelfig added this pull request to the merge queue Jun 21, 2023
Merged via the queue into master with commit c50674b Jun 21, 2023
@michaelfig michaelfig deleted the mfig-bridge-no-error-stack branch June 21, 2023 01:00
mhofman pushed a commit that referenced this pull request Aug 7, 2023
fix(cosmos): prevent Golang error wrapping stack frame divergence
mhofman pushed a commit that referenced this pull request Aug 7, 2023
fix(cosmos): prevent Golang error wrapping stack frame divergence
mhofman pushed a commit that referenced this pull request Aug 16, 2023
fix(cosmos): prevent Golang error wrapping stack frame divergence
mhofman pushed a commit that referenced this pull request Aug 16, 2023
fix(cosmos): prevent Golang error wrapping stack frame divergence
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't propagate Golang error stack information over the SwingSet bridge
2 participants