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

Add a unit test to cover JSON ack serialization non-determinism #8072

Closed
rootulp opened this issue Feb 28, 2025 · 0 comments · Fixed by #8073
Closed

Add a unit test to cover JSON ack serialization non-determinism #8072

rootulp opened this issue Feb 28, 2025 · 0 comments · Fixed by #8073

Comments

@rootulp
Copy link
Contributor

rootulp commented Feb 28, 2025

Context

GHSA-jg6f-48ff-5xrw

v7.8.0...v7.9.2#diff-8ddf47fbc8c8f270342c829d2e405b29d6b4732afeac6dd7ae1c22d5447abd4fR243-R246

Problem

The unit tests in that diff don't actually cover the fix. The tests pass even with these lines commented out.

Proposal

Add a test case that actually covers the fix. For example:

		{
			// See https://github.com/cosmos/ibc-go/security/advisories/GHSA-jg6f-48ff-5xrw
			"non-deterministic JSON ack serialization should return an error",
			func() {
				// Create a valid acknowledgement using deterministic serialization.
				ack = channeltypes.NewResultAcknowledgement([]byte{byte(1)}).Acknowledgement()
				// Introduce non-determinism: insert an extra space after the first character '{'
				// This will deserialize correctly but fail to re-serialize to the expected bytes.
				if len(ack) > 0 && ack[0] == '{' {
					ack = []byte("{ " + string(ack[1:]))
				}
			},
			errors.New("acknowledgement did not marshal to expected bytes"),
			false,
		},

See celestiaorg#1

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 a pull request may close this issue.

1 participant