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

Test compatibility of SplitMessages with/without share index #632

Closed
rootulp opened this issue Aug 18, 2022 · 2 comments · Fixed by #637
Closed

Test compatibility of SplitMessages with/without share index #632

rootulp opened this issue Aug 18, 2022 · 2 comments · Fixed by #637

Comments

@rootulp
Copy link
Collaborator

rootulp commented Aug 18, 2022

Context

During non-interactive defaults, we added empty message shares that act as padding between messages. In order for transactions to be linked to the share(s) that they pay for, indexes were added to SplitMessages.

Problem Definition

Since indexes didn't exist in v0.6.0, we need to verify if v0.6.0 is compatible with main.

Ref:

Proposal

Write a unit test or integ test that verifies compatibility of SplitMessages with/without share index

@evan-forbes
Copy link
Member

to expand on this slightly, the current tests do test comparability of the encoding and decoding, as we haven't changed them in #627 and won't be changing them when introducing non-interactive defaults either.

What we need to add is a mechanism to detect that we are decoding an old block, so that we don't add any padding or care if there is a share index in the wrapped transactions. This could be as simple (and implicit) as reading the first share index from a wrapped transaction. If that index is 0, then we automatically know that we are reading an old block, and can pass nil to SplitMessages(nil, msgs), which effectively uses identical share splitting code to v0.6.0

@evan-forbes
Copy link
Member

ref #608

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

Successfully merging a pull request may close this issue.

2 participants