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

Migrate old attestations #979

Merged
merged 12 commits into from
Apr 27, 2023

Conversation

Neylix
Copy link
Member

@Neylix Neylix commented Apr 17, 2023

Description

This issue handle the migration from old transaction summary and replication attestation to the new ones added by #941

It adds a migration script to transform old summary aggregate and summary with the new transaction summary containing the validation stamp checksum

Also migrate the current slots stored in summary cache with the new version.

Slot stored in subset is transformed only when an other process request it

Fixes #956

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

First need to create a release on version 1.0.7.
Do do so, go on develop and remove the commits from pr #941 and #927, you can do an interactive rebase git rebase -i HEAD~30 and drops the following commits

image

Then runs these commands :

# create the 1.0.7 release
./scripts/test-release.sh -i
# remove release artefact
git reset --hard
# switch to pr branch 
git switch Migrate-old-attestations

Here you can start the 2 nodes created:
./test_release_node1/bin/archethic_node console
ARCHETHIC_CRYPTO_SEED=node2 ARCHETHIC_P2P_PORT=3003 ARCHETHIC_HTTP_PORT=4001 ARCHETHIC_HTTPS_PORT=5001 ./test_release_node2/bin/archethic_node console

Now you need to prepare the branch to the new version. You have to update the version number and also remove a code change that will fail (as we already run the new version)
You can apply this patch to do so :
patch.txt
git apply patch.txt
(You can commit this patch to avoid applying it each time you switch branch)

Create the new release 1.0.8
./scripts/test-release.sh -p

You can wait the 2 nodes are authorized and create some summary aggregate.

Then execute the new upgrade
./scripts/test-release.sh -u

The upgrade should pass successfully without error, you can access to old summaries using the explorer (make sure the data is not cached)

Next summary time should create summary without error (You may have error with StatsCollector which was not started by default in the appup file)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Neylix Neylix added beacon chain Involve BeaconChain enhancements release Involve release / hot reload mecanism labels Apr 17, 2023
Transform a TransactionSummary without version into a TransactionSummary v1
"""
@spec transform_1_0_8_summary(t()) :: t()
def transform_1_0_8_summary(tx_summary = %__MODULE__{version: 1}), do: tx_summary
Copy link
Member

Choose a reason for hiding this comment

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

Why is it in this file ? I think the migration should have this function, hence do no need to alter this file after the upgrade, so the function will be contextualized for the migration only.

Copy link
Member

@samuelmanzanera samuelmanzanera Apr 21, 2023

Choose a reason for hiding this comment

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

I think it would be clearer to use a Slot.transform/2 and TransactionSummary.transform/2 which just do transformation of data structure for a given version.

Copy link
Contributor

@bchamagne bchamagne left a comment

Choose a reason for hiding this comment

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

I tested with the given instructions.

When I applied the release 1.0.8, this error started to pop every minute:

Screenshot_2023-04-25_16-03-06

But the nodes were working fine I believe, I tried to do a few transfer and no issue.
But then I opened the beacon explorer and impossible to read a summary that happened before the release 1.0.8.
The explorer display loading

Screenshot_2023-04-25_16-20-55

and the nodes display this error

Screenshot_2023-04-25_16-19-58

Hope it helps

@Neylix Neylix force-pushed the Migrate-old-attestations branch from 093e93f to 4715913 Compare April 26, 2023 12:45
Copy link
Contributor

@bchamagne bchamagne left a comment

Choose a reason for hiding this comment

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

tested with new changes, migration worked fine

@Neylix Neylix merged commit 24de8b0 into archethic-foundation:develop Apr 27, 2023
@Neylix Neylix deleted the Migrate-old-attestations branch April 27, 2023 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beacon chain Involve BeaconChain enhancements release Involve release / hot reload mecanism
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate old self repair / replication attestation to the new implementation
3 participants