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

feat: write on database the number of retries per certificate and the certificates in a history table #208

Merged
merged 10 commits into from
Dec 2, 2024
Merged

feat: write on database the number of retries per certificate and the certificates in a history table #208

merged 10 commits into from
Dec 2, 2024

Conversation

joanestebanr
Copy link
Contributor

@joanestebanr joanestebanr commented Nov 28, 2024 •
edited
Loading

Description

Configuration

[AggSender]
KeepCertificatesHistory = true

Next steps

  • Define the retain policy

@Stefan-Ethernal Stefan-Ethernal changed the title feat: cdk611 keep reties on storage feat: keep retries on storage Nov 28, 2024
@joanestebanr joanestebanr self-assigned this Nov 28, 2024
@joanestebanr joanestebanr force-pushed the feature/cdk611-keep_reties_on_storage branch from 80d5f0a to c21ebdf Compare November 28, 2024 14:00
@joanestebanr joanestebanr changed the title feat: keep retries on storage feat: write on database the number of retries per certificate and the certificates in a history table Nov 28, 2024
@joanestebanr joanestebanr marked this pull request as ready for review November 28, 2024 16:20
@leovct @joanestebanr
chore: bump kurtosis-cdk version to support pre-deployed gas token (#202
200331b 
)

* chore: kurtosis-cdk bump (gas token update)

* test

* chore: use new kurtosis-cdk tag

* chore: nit

* chore: clean up
@Stefan-Ethernal Stefan-Ethernal force-pushed the feature/cdk611-keep_reties_on_storage branch from c341557 to 200331b Compare November 29, 2024 08:49
ToniRamirezM
ToniRamirezM approved these changes Nov 29, 2024
View reviewed changes
aggsender/db/aggsender_db_storage.go Outdated Show resolved Hide resolved
@joanestebanr @ToniRamirezM
feat: PR comment
067fcb4 
Co-authored-by: Toni Ramírez <[email protected]>
vcastellm
vcastellm reviewed Nov 29, 2024
View reviewed changes
Copy link
Contributor

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

What's the difference between certificate_info_history and certificate_info? Could just the certificate_info table contain all records and filter by status? both table have the same data model

@joanestebanr
Copy link
Contributor Author

What's the difference between certificate_info_history and certificate_info? Could just the certificate_info table contain all records and filter by status? both table have the same data model

The idea is that certificate_info_history could be empty if KeepCertificatesHistory is false.
The table certificate_info enforce the integrity of data having as primary key height but the table with history is height and retry

vcastellm
vcastellm approved these changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

LGTM aside from minor comment

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Dec 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
84.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@joanestebanr joanestebanr merged commit f792176 into release/v0.5.0 Dec 2, 2024
12 checks passed
@joanestebanr joanestebanr deleted the feature/cdk611-keep_reties_on_storage branch December 2, 2024 15:46
@joanestebanr joanestebanr mentioned this pull request Dec 2, 2024
Merged
Vui-Chee added a commit to okx/xlayer-cdk that referenced this pull request Dec 3, 2024
@Vui-Chee
Merge tag 'v0.5.0-beta6' into vui-chee/basedev-upstream-v0.5.0-beta6
83a3e9e 
* tag 'v0.5.0-beta6':
  feat: write on database the number of retries per certificate and the certificates in a history table (0xPolygon#208)
  feat: return an error in case agglayer returns certificate with height lower than in local storage (0xPolygon#209)
  chore: simplify the `HashMeddler` (0xPolygon#205)
  fix: clean proof table on start (0xPolygon#207) (0xPolygon#210)
  feat: improve logs (0xPolygon#204)
  fix: cdk603 error calculating previousLocalExitRoot (0xPolygon#199)
  fix: Integration Bali PP (0xPolygon#198)
  feat: check agglayer certificate and use as initial if db is empty (0xPolygon#192)
  feat: sqlite aggregator (0xPolygon#189)
  feat: BridgeMessage e2e test (0xPolygon#184)
  feat: aggsender e2e (0xPolygon#183)
  fix: aggregating proofs (0xPolygon#191) (0xPolygon#193)
  feat: l1infotreesync can be run as individual component (0xPolygon#188)
  fix: l1infotree flaky test (0xPolygon#182)
  fix: `L1InfoRootIncorrect` error from `agglayer` (0xPolygon#185)
  feat: improve aggsender logs (0xPolygon#186) (0xPolygon#187)
  feat: remove sanity check (0xPolygon#178) (0xPolygon#179)
  refact: GetSequence method (0xPolygon#169)
  feat: epoch notifier (0xPolygon#144)
  feat: unpack and log agglayer errors (0xPolygon#158)
Stefan-Ethernal pushed a commit that referenced this pull request Dec 3, 2024
@joanestebanr
feat: cherry-pick PR #208 from release/v0.5.0 (#215)
e254ec9
Vui-Chee added a commit to okx/xlayer-cdk that referenced this pull request Dec 4, 2024
@Vui-Chee
Merge branch 'dev' into zhangkai/fix-senderaddr-ethman
93412a4 
* dev: (22 commits)
  fix issues
  feat: healthcheck (#11)
  feat: write on database the number of retries per certificate and the certificates in a history table (0xPolygon#208)
  feat: return an error in case agglayer returns certificate with height lower than in local storage (0xPolygon#209)
  chore: simplify the `HashMeddler` (0xPolygon#205)
  fix: clean proof table on start (0xPolygon#207) (0xPolygon#210)
  feat: improve logs (0xPolygon#204)
  fix: cdk603 error calculating previousLocalExitRoot (0xPolygon#199)
  fix: Integration Bali PP (0xPolygon#198)
  feat: check agglayer certificate and use as initial if db is empty (0xPolygon#192)
  feat: sqlite aggregator (0xPolygon#189)
  feat: BridgeMessage e2e test (0xPolygon#184)
  feat: aggsender e2e (0xPolygon#183)
  fix: aggregating proofs (0xPolygon#191) (0xPolygon#193)
  feat: l1infotreesync can be run as individual component (0xPolygon#188)
  fix: l1infotree flaky test (0xPolygon#182)
  fix: `L1InfoRootIncorrect` error from `agglayer` (0xPolygon#185)
  feat: improve aggsender logs (0xPolygon#186) (0xPolygon#187)
  feat: remove sanity check (0xPolygon#178) (0xPolygon#179)
  refact: GetSequence method (0xPolygon#169)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vcastellm vcastellm vcastellm approved these changes

@ToniRamirezM ToniRamirezM ToniRamirezM approved these changes

@goran-ethernal goran-ethernal goran-ethernal approved these changes

Assignees

@joanestebanr joanestebanr

Labels
aggsender config
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@joanestebanr @vcastellm @ToniRamirezM @goran-ethernal @leovct @Stefan-Ethernal