Skip to content

fix: fetch_owned should not include reserved#649

Open
jakmeier wants to merge 1 commit intosig-net:developfrom
jakmeier:fix_fetch_owned
Open

fix: fetch_owned should not include reserved#649
jakmeier wants to merge 1 commit intosig-net:developfrom
jakmeier:fix_fetch_owned

Conversation

@jakmeier
Copy link
Contributor

Reserved triple ids are not assigned to a owner, yet. Including them is inconsistent.

There is also a race condition, where a triple is already stored with another owner but unreserve has not been called, yet. during that time, a triple will be returned for two different owners.

For Presignatures, the owner is known ahead of time. But it is still wrong to assume that all reserved ids will be owned by this node.

Reserved triple ids are not assigned to a owner, yet.
Including them is inconsistent.

There is also a race condition, where a triple is already stored
with another owner but `unreserve` has not been called, yet.
during that time, a triple will be returned for two different owners.

For Presignatures, the owner is known ahead of time. But it is still
wrong to assume that all reserved ids will be owned by this node.
@jakmeier
Copy link
Contributor Author

@ChaoticTempest Not quite sure why you included reserved ids in the first place. I might be missing something.

But it messes with the fixture generating tests, which read out all owned triples / presignatures of a node and dump them into JSON files. Those JSON files then contain conflicts.

@ChaoticTempest
Copy link
Contributor

it's because of the race condition with state sync. It uses fetch_owned to see which items we own. On owner take, if state sync is currently in progress, then the non-owners would see that we no longer "own" the shares and delete it. We don't have to put reserved in with fetch_owned now that reserved is in memory, but state sync should now do a union over reserverd and owned

@jakmeier
Copy link
Contributor Author

jakmeier commented Feb 5, 2026

Ah right, thanks @ChaoticTempest! Now I understand why the union is there.

It would be the cleanest, if we finish the implementation of state sync according to the spec.

The case here would be solved cleanly if we map reserved ids to the "Generating" state. This state should be included in the state sync, so the peer can make the right decision on their end.

On that note, we also still lack the two way sync. Meaning, we have no idea if nodes actually have a P or T when we select them as participant.

@volovyks volovyks mentioned this pull request Feb 9, 2026
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 this pull request may close these issues.

2 participants