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 #2175 - Add SignatureCarrier interface for transactions with signatures #2189

Closed

Conversation

pri-3x
Copy link

@pri-3x pri-3x commented Sep 30, 2024

Relates to issue #2175

-Add SignatureCarrier interface for improved transaction signature handling.

@@ -57,7 +57,7 @@ func TestTransactionCommitmentPoseidon(t *testing.T) {

c, err := transactionCommitmentPoseidon(txs)
require.NoError(t, err)
expected := utils.HexToFelt(t, "0x6e067f82eefc8efa75b4ad389253757f4992eee0f81f0b43815fa56135ca801")
expected := utils.HexToFelt(t, "0x5ecb75d7a86984ec8ef9d5fbbe49ef8737c37246d33cf73037df1ceb412244e")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why the hash changed?

Copy link
Author

Choose a reason for hiding this comment

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

@derrix060 The hash changed due to the introduction of the SignatureCarrier interface, which altered how signatures are handled in the hash calculation. I got this hash, when I ran the test case.

Test:       	TestTransactionCommitmentPoseidon/txs_without_signature
            	Messages:   	expected: 0x6e067f82eefc8efa75b4ad389253757f4992eee0f81f0b43815fa56135ca801, 
            	got: 0x5ecb75d7a86984ec8ef9d5fbbe49ef8737c37246d33cf73037df1ceb412244e

Copy link
Contributor

This pull request is stale because it has been open 35 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 10, 2024
@kirugan kirugan closed this Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants