FIP-0109: Open up DDO notifications to user contracts #1689

ZenGround0 · 2025-07-09T15:03:56Z

Draft until we get a FIP draft up

rvagg · 2025-07-10T01:55:10Z

https://hackmd.io/jpWwEcjRQrGnjyMvRmDNiw, we could just post that and get the ball rolling if you think it's worth it. My concern was mainly that without a SectorStatus this doesn't buy us very much and has some risk associated with it; and also that a SectorStatus would be much higher value even on its own. No reason we can't do both though.

ZenGround0 · 2025-07-10T07:01:49Z

Given how easy this one is I'm all for it even if we unfortunately can't get sector status in. At this point the risks are not significant enough to hold us up (unless you know something I don't)

rvagg · 2025-07-24T09:01:59Z

Posted FIP for this @ filecoin-project/FIPs#1180

BigLep · 2025-08-12T03:14:08Z

@ZenGround0 @rvagg : who is owning getting this ready for review? (apologies if this was communicated and I missed it)

codecov-commenter · 2025-08-28T03:38:16Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.23%. Comparing base (00db828) to head (3ceec26).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1689      +/-   ##
==========================================
+ Coverage   91.20%   91.23%   +0.02%     
==========================================
  Files         151      151              
  Lines       32014    32006       -8     
==========================================
+ Hits        29199    29200       +1     
+ Misses       2815     2806       -9

Files with missing lines	Coverage Δ
actors/miner/src/notifications.rs	`83.60% <ø> (+3.60%)`	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ZenGround0 · 2025-08-29T01:19:58Z

actors/evm/tests/contracts/NotificationReceiver.sol

+        return getCBORData(ret_acc);
+    }
+
+    /* *** CBOR parsing *** */


Given a bit more time I would have externalized these somewhere for reuse, but I'll leave that for the future.

https://github.com/filecoin-project/filecoin-solidity is where these can live; I mentioned in the FIP that we should put some of this stuff in there eventually.

https://github.com/filecoin-project/filecoin-solidity/blob/master/contracts/v0.8/cbor/MinerCbor.sol could probably get a SectorContentChanged helper

https://github.com/filecoin-project/filecoin-solidity is where these can live

Yeah I took a lot of these from there. By externalize I only meant within builtin actor testing context. I'm not thinking we want to set up a proper build environment and checking in full libraries as a dependency in builtin actors. But maybe there is a happy medium where we have a set of small self contained library contracts checked in here that we use in a library fashion in testing contracts.

proper build environment

agreed, upstreaming stuff from here would be the goal though, we can play it verbose here

ZenGround0 · 2025-08-29T01:20:25Z

actors/evm/tests/contracts/NotificationReceiver.sol

+     *
+     * All notifications are accepted so CBOR true returned for every piece of every notified sector
+     */
+    function processSectorContentChanged(bytes memory params) internal returns (bytes memory) {


A nice thing about this test is that this is a little reference implementation for how to handle DDO notifications

we should pull some of this back into the FIP where there's already some example code

rvagg · 2025-08-29T07:18:48Z

@ZenGround0 could we try out the "isMinerActor" approach suggested in the FIP @ https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0109.md#example-solidity-contract-interface ? It would be great to see if we can verify that actually working in here and if we're going to point people here for the example then it would be good for it to be complete in that way and not steer them wrong to deploy insecure contracts.

integration_tests/src/tests/evm_notification_test.rs

rvagg · 2025-08-29T07:28:47Z

Exciting! I can think of lots of permutations of this that would be good to test - multiple sectors, mixed sectors with f05 and other multiple contracts, failures, etc. but the list is pretty long when you stretch it out. I think you said you'd be doing some more unit tests that might cover more of these scenarios so maybe that OK?

Update status of FIP-0109 Incorporate relevant notes / comments from working through the implementation in filecoin-project/builtin-actors#1689

BigLep · 2025-08-29T15:19:32Z

FYI I started filecoin-project/FIPs#1188 as a place to add any FIP updates from doing this work.

BigLep · 2025-09-01T15:18:55Z

Note that validating this work in Lotus will be tracked in filecoin-project/lotus#13286

actors/evm/tests/contracts/NotificationReceiver.sol

Co-authored-by: Rod Vagg <[email protected]>

ZenGround0 · 2025-09-03T07:06:03Z

integration_tests/src/tests/evm_notification_test.rs

+    .abi_encode();
+
+    // Attempt to invoke the contract method from a regular account (not a miner)
+    let call_result = v.execute_message(


Please note that I compiled the NotificationReciever contract with isMinerActor check disabled and when instantiating evm actor with that hex this test fails. This makes me confident that the failure is actually in the isMinerActor check not somewhere else along the way.

rvagg · 2025-09-03T08:20:30Z

actors/evm/tests/contracts/NotificationReceiver.sol

+
+    }
+
+    function isIDAddress(address _a) internal pure returns (bool isID, uint64 id) {


this is a clever function, we have no other indicator of id addresses at this point do we, like an initial byte or something? we're banking on nobody being able to stumble across a wallet that resolves to an address that happens to be all leading zeros and whose last bytes collide with an existing miner actor eh? or am I missing some other property of what we have at this point?

This is a really good point. What the evm actor is doing when it sets msg.sender is:

tries to resolve an addr to f4 and then use the delegated 20 byte payload to get to eth style

if that fails then encode the id in the eth payload
The the evm actor's code has no way to differentiate. At least that I can see after staring at fvm precompiles.

Right now we are totally reliant on the fact that getting so many 0s in an eth key is actually computationally infeasible. In the worst case where miner ids are actually 64 bits that's only 96 bits of security. Since collisions are not the problem direct search is this is "fine" for a security model that doesn't include state actors or quantum computers. In practice its better than 128 bits of security because we would need to have ~trillions of actors added to the network to get miner ids this high which is going to run into bottlenecks elsewhere.

But I don't like needing to think all this through. What we need is a simple actor type precompile that uses the fvm runtime's function. Such a precompile would be trivial to add.

integration_tests/src/tests/evm_notification_test.rs

rvagg · 2025-09-03T09:02:50Z

actors/evm/tests/contracts/NotificationReceiver.sol

+    uint64 constant SECTOR_CONTENT_CHANGED = 2034386435;
+
+    /**
+     * @dev Get the count of notifications for a specific sector


sol! is picking up the javadoc style /** comments and @dev is being thrown into the mix when the macro is expanded; if you just replace all these comments with // then the tests pass I think

tests in CI that is, see failure there

Thanks, failure to check AI on my part

rvagg

lgtm, plenty of other permutations we could test but given how much effort it is just to do this it's probably a blood-from-stone situation

Co-authored-by: Rod Vagg <[email protected]>

github-project-automation bot added this to FilOz Jul 9, 2025

github-project-automation bot moved this to 📌 Triage in FilOz Jul 9, 2025

BigLep moved this from 📌 Triage to ⌨️ In Progress in FilOz Jul 15, 2025

BigLep assigned ZenGround0 Jul 23, 2025

BigLep changed the title ~~Open up DDO notifications to user contracts~~ FIP-0109: Open up DDO notifications to user contracts Aug 12, 2025

BigLep added this to nv27 Track Board Aug 15, 2025

github-project-automation bot moved this to 🐱 Todo in nv27 Track Board Aug 15, 2025

ZenGround0 force-pushed the feat/real-ddo branch from 18ea456 to a2909fe Compare August 28, 2025 03:27

ZenGround0 force-pushed the feat/real-ddo branch from a2909fe to b38aefe Compare August 28, 2025 04:33

BigLep moved this from 🐱 Todo to ⌨️ In Progress in nv27 Track Board Aug 28, 2025

ZenGround0 commented Aug 29, 2025

View reviewed changes

ZenGround0 marked this pull request as ready for review August 29, 2025 01:46

rvagg reviewed Aug 29, 2025

View reviewed changes

integration_tests/src/tests/evm_notification_test.rs Outdated Show resolved Hide resolved

BigLep added a commit to filecoin-project/FIPs that referenced this pull request Aug 29, 2025

FIP-0109: set to Accepted and update notes

add4639

Update status of FIP-0109 Incorporate relevant notes / comments from working through the implementation in filecoin-project/builtin-actors#1689

BigLep mentioned this pull request Aug 29, 2025

FIP-0109: set to Accepted and update notes filecoin-project/FIPs#1188

Draft

BigLep mentioned this pull request Sep 1, 2025

Validate FIP-0109 (deferred until nv28) filecoin-project/lotus#13286

Open

ZenGround0 added 4 commits September 2, 2025 15:02

Remove market actor requirement on notifications

bc5d352

claude first pass

e918fb0

Integration test fixes

754dc0c

One contract fix and attempt at wiring integration test

6100f84

Review Response: check before and afte

02d418d

ZenGround0 force-pushed the feat/real-ddo branch from e42a276 to 02d418d Compare September 2, 2025 21:02

ZenGround0 added 3 commits September 2, 2025 21:58

Attempt isMiner actor receiver contract

d785213

Correct hex

d58fcb4

Fix filecoin actor call bug

3068965

ZenGround0 requested a review from rvagg September 3, 2025 05:52

ZenGround0 added 2 commits September 3, 2025 08:40

Test that isMiner check rejects diret calls

246dc48

fmt

061547b

rvagg reviewed Sep 3, 2025

View reviewed changes

actors/evm/tests/contracts/NotificationReceiver.sol Show resolved Hide resolved

ZenGround0 and others added 2 commits September 3, 2025 09:02

clippy

9a90ab0

Update actors/evm/tests/contracts/NotificationReceiver.sol

5fa2c4e

Co-authored-by: Rod Vagg <[email protected]>

ZenGround0 commented Sep 3, 2025

View reviewed changes

rvagg reviewed Sep 3, 2025

View reviewed changes

integration_tests/src/tests/evm_notification_test.rs Outdated Show resolved Hide resolved

rvagg reviewed Sep 3, 2025

View reviewed changes

integration_tests/src/tests/evm_notification_test.rs Show resolved Hide resolved

rvagg reviewed Sep 3, 2025

View reviewed changes

rvagg approved these changes Sep 3, 2025

View reviewed changes

github-project-automation bot moved this from ⌨️ In Progress to ✔️ Approved by reviewer in FilOz Sep 3, 2025

ZenGround0 and others added 6 commits September 3, 2025 12:02

Update integration_tests/src/tests/evm_notification_test.rs

5b105e7

Co-authored-by: Rod Vagg <[email protected]>

Update integration_tests/src/tests/evm_notification_test.rs

f800c8c

Co-authored-by: Rod Vagg <[email protected]>

Tweak solidity test file for codecov to not fail

aac56d5

fmt

7e1d627

Fixup review suggestion

6217cbd

Codecov is extremely picky

3ceec26

ZenGround0 added this pull request to the merge queue Sep 3, 2025

Merged via the queue into master with commit a3b0adf Sep 3, 2025
12 checks passed

ZenGround0 deleted the feat/real-ddo branch September 3, 2025 17:05

github-project-automation bot moved this from ⌨️ In Progress to 🎉 Done in nv27 Track Board Sep 3, 2025

github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Sep 3, 2025


		}

		function isIDAddress(address _a) internal pure returns (bool isID, uint64 id) {

FIP-0109: Open up DDO notifications to user contracts #1689

FIP-0109: Open up DDO notifications to user contracts #1689

Uh oh!

Conversation

ZenGround0 commented Jul 9, 2025

Uh oh!

rvagg commented Jul 10, 2025

Uh oh!

ZenGround0 commented Jul 10, 2025

Uh oh!

rvagg commented Jul 24, 2025

Uh oh!

BigLep commented Aug 12, 2025

Uh oh!

codecov-commenter commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZenGround0 Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rvagg commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

rvagg commented Aug 29, 2025

Uh oh!

BigLep commented Aug 29, 2025

Uh oh!

BigLep commented Sep 1, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rvagg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Aug 28, 2025 •

edited

Loading

ZenGround0 Sep 2, 2025 •

edited

Loading

rvagg commented Aug 29, 2025 •

edited

Loading