Skip to content

Sector status fixes and solidity integration test#1720

Open
ZenGround0 wants to merge 8 commits intomasterfrom
test/sector-status-integration
Open

Sector status fixes and solidity integration test#1720
ZenGround0 wants to merge 8 commits intomasterfrom
test/sector-status-integration

Conversation

@ZenGround0
Copy link
Copy Markdown
Contributor

This is a follow on to the main PR introducing sector status #1707. This follows the pattern from #1689 and introduces a solidity integration test. This appears to work but had a lot of ai in the loop and needs more validation and simplification. Additionally I think we can clean up our previous itest to extract the shared cbor and calling logic into a library file.

@github-project-automation github-project-automation bot moved this to 📌 Triage in FilOz Feb 24, 2026
@ZenGround0 ZenGround0 marked this pull request as ready for review February 24, 2026 22:19
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.53%. Comparing base (f4f7a36) to head (7fd8716).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1720   +/-   ##
=======================================
  Coverage   90.53%   90.53%           
=======================================
  Files         139      139           
  Lines       27693    27693           
=======================================
  Hits        25072    25072           
  Misses       2621     2621           
Files with missing lines Coverage Δ
actors/miner/src/types.rs 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ZenGround0 ZenGround0 requested a review from Kubuxu March 3, 2026 15:37
@BigLep BigLep moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Mar 3, 2026
FilecoinCBOR.CBORBuffer memory buf = FilecoinCBOR.createCBOR(64 + statusBytes.length + auxData.length);
FilecoinCBOR.startFixedArray(buf, 3);
FilecoinCBOR.writeUInt64(buf, sectorNumber);
FilecoinCBOR.writeTextString(buf, status);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are enums transformed into text?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nice catch and apparently dag cbor writes rust enums as raw strings?! i.e. "Active" "Faulty" "Dead"

I'll move this to integer enums.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

At the same time I'll fullfil your request for reordering Dead as 0 @wjmelements

Base automatically changed from feat/sector-exposure to master March 17, 2026 18:39
@ZenGround0 ZenGround0 force-pushed the test/sector-status-integration branch from 4315cc9 to b3d7b31 Compare March 19, 2026 21:59
@ZenGround0 ZenGround0 changed the title Sector status solidity integration test Sector status fixes and solidity integration test Mar 19, 2026
Comment on lines +69 to 70
// 0x51 is IPLD CBOR codec
require(inCodec == 0x51, "Invalid codec");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can it be a constant?

Suggested change
// 0x51 is IPLD CBOR codec
require(inCodec == 0x51, "Invalid codec");
require(inCodec == FilecoinCBOR.CBOR_CODEC, "Invalid codec");

Comment on lines +70 to +71
require_activation_success: false,
require_notification_success: false,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should these be true?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah that's a bit cleaner

@wjmelements
Copy link
Copy Markdown

Claude identified a discrepancy in error handling with the FIP:

  FIP vs builtin-actors disagreement

  The FIP says for (NO_DEADLINE, NO_PARTITION) with Active or Faulty request:

  ▎ "these are trivially never found in this location and the validation method returns false"

  But builtin-actors (validate_at_no_location_errors_when_sector_in_amt) shows that if the sector IS in the AMT (any live sector), ALL requests — including Dead — return
  USR_NOT_FOUND error, not false. Only when the sector is absent from the AMT entirely does it return { valid: status == Dead }.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🔎 Awaiting Review

Development

Successfully merging this pull request may close these issues.

5 participants