Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Fix hold aspect bugs #2184

Merged
merged 46 commits into from
May 14, 2020
Merged

Fix hold aspect bugs #2184

merged 46 commits into from
May 14, 2020

Conversation

zippy
Copy link
Member

@zippy zippy commented May 7, 2020

PR summary

This PR fixes bugs in the conductor and sim2h with hold aspect:

  • two conductor fixes during error cases (like base hasn't arrived yet) in hold_aspects request:
    • stop incorrectly recording aspects as held
    • stop locking up the future
  • because sim2h doesn't get error messages from a hold_aspect request it must also not record aspects sent in those messages as held, and thus the conductor must explicitly send back a list of aspects held after a hold_aspect request

This fix requires putting in a holding attempt result into the state for the future to look for to be able to pass the result back, as well as making sure to be able to clean up that state data when the pending validation is cleared.

testing/benchmarking notes

( if any manual testing or benchmarking was/should be done, add notes and/or screenshots here )

followups

( any new tickets/concerns that were discovered or created during this work but aren't in scope for review here )

changelog

  • if this is a code change that effects some consumer (e.g. zome developers) of holochain core, then it has been added to our between-release changelog with the format
- summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)

documentation

@zippy zippy requested a review from lucksus May 7, 2020 19:47
Copy link
Collaborator

@Connoropolous Connoropolous left a comment

Choose a reason for hiding this comment

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

hey Eric,
I was reading this anyways and I get the sense things are a little sparse on the review side of things, so I figured I would lend a hand.

crates/core/src/dht/dht_reducers.rs Outdated Show resolved Hide resolved
crates/core/src/dht/dht_reducers.rs Outdated Show resolved Hide resolved
crates/core/src/dht/dht_reducers.rs Show resolved Hide resolved
crates/core/src/dht/dht_reducers.rs Show resolved Hide resolved
crates/core/src/dht/dht_store.rs Show resolved Hide resolved
crates/core/src/wasm_engine/api/commit.rs Outdated Show resolved Hide resolved
crates/core/src/dht/dht_store.rs Show resolved Hide resolved
@zippy zippy merged commit 249e31f into develop May 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants