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.

Sign up for GitHub

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

Jump to bottom

Fix bytecode hash calculation #1414

Merged
merged 18 commits into from
Jun 17, 2024
Merged

Fix bytecode hash calculation #1414

merged 18 commits into from
Jun 17, 2024

Conversation

marcocastignoli
Copy link
Member

@marcocastignoli marcocastignoli commented May 30, 2024 •
edited
Loading

This PR fixes the bytecode hash calculation, previously inverted.

This PR also:

  • adds tests to verify consistency of code.code_hash
  • prevent calling getTxReceipt twice during matchWithCreationTx
  • update migration script:
    • allow to pass specific chains to sync
    • when querying the database, retry when receiving a connection error with a delay

Caution

Do not merge this PR until the new database is up and running in staging!

@marcocastignoli
Fix bytecode hash calculation by updating variable names
17f14c1 
- Add tests to verify consistency of code.code_hash
- Prepare database with new tables and triggers for 'inverted-code-hashes’ fix process
@marcocastignoli marcocastignoli changed the title Fix bytecode hash calculation by updating variable names Fix bytecode hash calculation May 30, 2024
@marcocastignoli marcocastignoli self-assigned this May 30, 2024
manuelwedler
manuelwedler requested changes Jun 5, 2024
View reviewed changes
Copy link
Collaborator

@manuelwedler manuelwedler left a comment

Choose a reason for hiding this comment

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

Mainly some improvement to the testing and clarification regarding the second database service.

services/server/test/integration/database.spec.ts Outdated Show resolved Hide resolved
services/server/test/integration/database.spec.ts Outdated Show resolved Hide resolved
services/server/src/server/services/StorageService.ts Show resolved Hide resolved
packages/lib-sourcify/src/lib/SourcifyChain.ts Outdated Show resolved Hide resolved
packages/lib-sourcify/src/lib/verification.ts Show resolved Hide resolved
This was referenced Jun 6, 2024
Closed
Closed
manuelwedler
manuelwedler approved these changes Jun 6, 2024
View reviewed changes
Copy link
Collaborator

@manuelwedler manuelwedler left a comment

Choose a reason for hiding this comment

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

Looks good :)

@marcocastignoli
Copy link
Member Author

I'm going to create a new PR based on this to fix #1419.

Also, let's wait to merge this until we release.

@marcocastignoli marcocastignoli merged commit e1ef411 into staging Jun 17, 2024
5 of 6 checks passed
@kuzdogan kuzdogan deleted the fix-inverted-code-hashes branch July 29, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BlackLeopardA2 BlackLeopardA2 BlackLeopardA2 approved these changes

@manuelwedler manuelwedler manuelwedler approved these changes

Assignees

@marcocastignoli marcocastignoli

Labels
None yet
Projects
Sourcify Public
Status: COMPLETED
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marcocastignoli @manuelwedler @BlackLeopardA2