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

Add ability to validate entries with full chain validation when author is offline #1932

Merged

Conversation

willemolding
Copy link
Collaborator

@willemolding willemolding commented Dec 2, 2019

PR summary

Finally adds ability to validate ChainFull entries when the author is offline (with tests to prove it). This is essential for correct operation of HoloFuel.

  • Adds timeouts for GetValidationPackage actions. Without this it is unable to fallback to restoring the authors chain from the headers
  • Adds a three staged approach in validation_package. First tries to build locally, then tries to message the author to get their chain directly. Finally rebuilds the authors chain from the DHT and validates against that.
  • Adds that entries are dependent on the previous header entry already being valid. Technically this is only required in the ChainFull validation case but it doesn't make the distinction at this time

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

willemolding and others added 30 commits July 29, 2019 12:54
…olochain/holochain-rust into fallback-build-validation-packages-dht
…olochain/holochain-rust into fallback-build-validation-packages-dht
@lucksus lucksus requested a review from zippy December 6, 2019 15:52
@lucksus lucksus dismissed zippy’s stale review December 6, 2019 15:53

Dismiss change-request as per message: #1932 (comment)

@lucksus lucksus changed the base branch from add-smarter-pending-validation-resolution to develop December 6, 2019 16:43
@maackle
Copy link
Collaborator

maackle commented Dec 6, 2019

So are we saying that the explicit publish should have no effect, and that it's only there for the sake of the consistency model?

@lucksus
Copy link
Collaborator

lucksus commented Dec 6, 2019

I have reran CI now several times. There is one failing app-spec assertion with sim1h which fails consistently. It is neither the CI timeout-flakyness nor the port issue. I think we have to investigate this. Could be the header publishing. This is the failure log:

  not ok 96 should be equivalent
⨯ g...    ---
      operator: deepEqual
      expected: |-
        [ 'HcSCJ965aADATg7ksqHTea4R5crA9evmv6k8PPaD6t5zfhzj6f87tb8ZfHAdmbz', 'HcSCJ965aADATg7ksqHTea4R5crA9evmv6k8PPaD6t5zfhzj6f87tb8ZfHAdmbz', 'HcSCj545ecJRQDnbstxD4tYrMRByo8d5xeKb8Z6o94Pn9TconEWgG94iXUvf9bz', 'HcScIeB7Qf6ik9pp33iUc6DM5CzyhnppfMU4Q4zejpm637ivOi43H9kYmSSgftz' ]
      actual: |-
        [ 'HcSCJ965aADATg7ksqHTea4R5crA9evmv6k8PPaD6t5zfhzj6f87tb8ZfHAdmbz', 'HcSCJ965aADATg7ksqHTea4R5crA9evmv6k8PPaD6t5zfhzj6f87tb8ZfHAdmbz', 'HcSCj545ecJRQDnbstxD4tYrMRByo8d5xeKb8Z6o94Pn9TconEWgG94iXUvf9bz' ]
      at: <anonymous> (/root/project/app_spec/test/files/entry.js:235:7)
      stack: |-
        Error: should be equivalent
            at Test.assert [as _assert] (/root/project/app_spec/test/node_modules/tape/lib/test.js:225:54)
            at Test.bound [as _assert] (/root/project/app_spec/test/node_modules/tape/lib/test.js:77:32)
            at Test.tapeDeepEqual (/root/project/app_spec/test/node_modules/tape/lib/test.js:422:10)
            at Test.bound [as deepEqual] (/root/project/app_spec/test/node_modules/tape/lib/test.js:77:32)
            at /root/project/app_spec/test/files/entry.js:235:7
            at runMicrotasks (<anonymous>)
            at processTicksAndRejections (internal/process/task_queues.js:93:5)

@maackle
Copy link
Collaborator

maackle commented Dec 6, 2019

Hmm, I ran it several times locally and it passed. I wonder why a change to core would cause sim1h to fail and not sim2h. I'll try restarting CI one more time.

@maackle
Copy link
Collaborator

maackle commented Dec 7, 2019

I got it to pass twice in CI, but most recently it failed again. So it is not a unilateral failure, but more flakiness. I suppose an increase in flakyness is a problem. But could this just be some artifact of sim1h? Rather than a problem with core?

@maackle
Copy link
Collaborator

maackle commented Dec 7, 2019

passthrough-dna's tests are also failing with sim1h. Seems like something is up with sim1h.

@willemolding
Copy link
Collaborator Author

I have removed the header publishing on init and everything is still working on the author list alone!

On the flaky sim1h test that test isn't one that I would have expected to be touched by this PR. I guess everything is so connected there is no way of knowing for sure but could this be unrelated flakyness?

@zippy zippy merged commit 70ed96c into develop Dec 9, 2019
@zippy zippy deleted the fallback-build-validation-packages-dht-with-smart-validation branch January 3, 2020 22:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
NEEDS REVIEW This PR requires peer code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants