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

Prover: fix the txnrlp duplicated txnrlp bug #80

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

AlexandreBelling
Copy link
Contributor

This PR implements a fix for an outstanding integration issue between the prover and txnrlp. The issue is that the prover infers a transaction signature via a mapping between tx-hash and tx-signature but it is possible that two transactions have the same transaction hash (as in the hash for signing). In that case, the current approach will not work. This PR addresses the issue by mapping the signature by tx-id.

The tx-hash is still used to sanity-check that the signature that is infered relates to the expected transaction. Thus, helping in the debugging process if any bugs stands out here.

@AlexandreBelling AlexandreBelling added the Prover Tag to use for all work impacting the prover label Sep 23, 2024
@AlexandreBelling AlexandreBelling self-assigned this Sep 23, 2024
Copy link

github-actions bot commented Sep 23, 2024

Delta Summary - Kotlin Code Coverage

Generated on: 09/23/2024 - 16:02
Description Previous Current Delta
Coverage date: 09/23/2024 - 16:02 09/23/2024 - 16:02
Tag: 362_10997807257 362_10997807257
Line coverage: 27.3% 27.3% 0.0%
Covered lines: 25578 25578 0
Coverable lines: 93680 93680 0
Total lines: 132748 132748 0
Branch coverage: 10.3% 10.3% 0.0%
Covered branches: 5494 5494 0
Total branches: 52848 52848 0
Method coverage: Feature is only available for sponsors

@AlexandreBelling AlexandreBelling merged commit 7a46988 into main Sep 23, 2024
25 checks passed
@AlexandreBelling AlexandreBelling deleted the prover/fix-txn-rlp-issue branch September 23, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prover Tag to use for all work impacting the prover
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants