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

Clarify market actor sector termination with expired deals #1417

Merged
merged 2 commits into from
May 18, 2021

Conversation

anorth
Copy link
Member

@anorth anorth commented May 17, 2021

#1411 described a possible problem with the market actor handling termination of sectors after deals expire. I determined it's not a problem, but clarified and cleaned up associated code and tests here. This shows that OnMinerSectorsTerminate does not abort if deals have expired. So this PR does not change actor behaviour.

The first commit here adds return values to some of the mock runtime's setter methods, so that a value (like an epoch) can be set and assigned to a local variable in one line: newValue = rt.SetEpoch(oldValue + delta).

Closes #1411.

@anorth anorth requested a review from ZenGround0 May 17, 2021 01:58
@codecov-commenter
Copy link

Codecov Report

Merging #1417 (47ae52f) into master (c7cff61) will increase coverage by 0.0%.
The diff coverage is 88.8%.

@@          Coverage Diff           @@
##           master   #1417   +/-   ##
======================================
  Coverage    69.8%   69.9%           
======================================
  Files          72      72           
  Lines        7792    7786    -6     
======================================
  Hits         5446    5446           
+ Misses       1451    1448    -3     
+ Partials      895     892    -3     

Copy link
Contributor

@ZenGround0 ZenGround0 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1448,6 +1436,7 @@ func TestCronTick(t *testing.T) {
rt.ExpectAbort(exitcode.ErrNotFound, func() {
actor.cronTick(rt)
})
//actor.checkState(rt) // cannot check state invariants
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this could be moved to assert the exact state invariants broken by calling CheckStateInvariants directly and asserting on the number and contents of the output messages.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@anorth anorth changed the title Add return values to mock runtime setters Clarify market actor sector termination with expired deals May 18, 2021
@anorth anorth force-pushed the anorth/fixtermination branch from 47ae52f to a77497c Compare May 18, 2021 05:29
@anorth anorth merged commit cef5d7e into master May 18, 2021
@anorth anorth deleted the anorth/fixtermination branch May 18, 2021 05:30
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.

Fix unnecessary sector termination failure
3 participants