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

Backward compatibility testing strategy #2151

Closed
lgalabru opened this issue Jun 4, 2023 · 7 comments
Closed

Backward compatibility testing strategy #2151

lgalabru opened this issue Jun 4, 2023 · 7 comments

Comments

@lgalabru
Copy link

lgalabru commented Jun 4, 2023

Hey @raphjaph, @veryordinally!
I just saw that v0.6.0 was released, pretty shortly after merging #2145, which a big PR where many things could have been broken.
What is the current strategy for making sure that the inscription number sequence produced by a version N is backward compatible with version N-1?
Are you guys re-indexing from scratch and comparing the sequence, and every single inscription?
As explained in this comment the way the fix for #2062 was approached in #2107 might not be backward compatible, as recognized here #2062 (comment)).

@BatBushRacks
Copy link

@lgalabru I agree... this seems like it was rushed while the devs continue to hide from the fact that users are upset about the lack of discourse on #2109... I truly hope it was done with care... hopefully no additional bugs...

@veryordinally
Copy link
Collaborator

Hey @raphjaph, @veryordinally! I just saw that v0.6.0 was released, pretty shortly after merging #2145, which a big PR where many things could have been broken. What is the current strategy for making sure that the inscription number sequence produced by a version N is backward compatible with version N-1? Are you guys re-indexing from scratch and comparing the sequence, and every single inscription? As explained in this comment the way the fix for #2062 was approached in #2107 might not be backward compatible, as recognized here #2062 (comment)).

Yes. We compare index from version - 1 to new version and made sure positive inscriptions don't change. We've been working on testing this for multiple weeks now. The issue you pointed out was indeed tricky, and we had not expected we'd run into it with this trimmed down PR, but we did, so we implemented a solution - which I think corresponds to what you suggested (don't really think there's another choice).

@lgalabru
Copy link
Author

lgalabru commented Jun 4, 2023

@veryordinally I'm not referring to re-inscribing cursed sats, I'm referring to the patch that @casey implemented for fixing the supertestnet issue. I think this patch is incorrect and breaking inscriptions numbers - only in 2 blocks.
Please open the links from my previous message :)

@veryordinally
Copy link
Collaborator

@lgalabru I agree... this seems like it was rushed while the devs continue to hide from the fact that users are upset about the lack of discourse on #2109... I truly hope it was done with care... hopefully no additional bugs...

Your critical words are appreciated. The decision to roll this out in stages was made for a set of reasons:

  • to make it easier for everyone to prepare for the rollout of cursed inscriptions and get infrastructure ready
  • to have enough time and bandwidth to address each type of cursed inscription thoroughly.

We do not consider this a rushed release, we have indeed been working on and testing related PRs for multiple weeks now and feel confident we have a pretty good handle on the changes.

@veryordinally
Copy link
Collaborator

@veryordinally I'm not referring to re-inscribing cursed sats, I'm referring to the patch that @casey implemented for fixing the supertestnet issue. I think this patch is incorrect and breaking inscriptions numbers - only in 2 blocks. Please open the links from my previous message :)

We compared the supertestnet inscriptions manually and had fixed the bug in Casey's patch. Can you send me a DM on twitter to sync?

@BatBushRacks
Copy link

@lgalabru I agree... this seems like it was rushed while the devs continue to hide from the fact that users are upset about the lack of discourse on #2109... I truly hope it was done with care... hopefully no additional bugs...

Your critical words are appreciated. The decision to roll this out in stages was made for a set of reasons:

* to make it easier for everyone to prepare for the rollout of cursed inscriptions and get infrastructure ready

* to have enough time and bandwidth to address each type of cursed inscription thoroughly.

We do not consider this a rushed release, we have indeed been working on and testing related PRs for multiple weeks now and feel confident we have a pretty good handle on the changes.

@veryordinally I guess already another bug came from it... #2145 (comment)... I guess fix in motion here #2154... please take care this time... no need to rush it...

@creamplanet
Copy link

Yikes seems like a lot of bugs come from inscription numbers. Are they really needed?

@casey casey closed this as completed Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants