-
Notifications
You must be signed in to change notification settings - Fork 43
getDealActivation(dealID) will revert the contract if the deal is expired #363
Comments
Hi, |
Hi @ottpeter In Filecoin if the deal is expired it will throw an error (see https://github.com/filecoin-project/builtin-actors/blob/master/actors/market/src/lib.rs#L979). Indeed the description should not indicate that it will return There is nothing that we can do in the lib for that. You could however in your code attempt to catch this error and verify it matches |
Hi @rllola |
The joy of solidity... Ok you might be able to bypass this by rewritting your own Just remove the The other option would be to have a call in the builtin-actor that allow to ask if a deal has expired or not. (It would be the best solution in my opinion). |
The problem is that the library is losing information along the way. The library needs to catch/lower the exception at the call site, parse the exit code, and propagate it as a return value. |
To be more concrete, the library should not unconditionally REVERT here: filecoin-solidity/contracts/v0.8/utils/Actor.sol Lines 123 to 125 in 461d993
Instead, it should parse the exit code from the data returned by the precompile, and propagate it back. |
See this thread on Slack: https://filecoinproject.slack.com/archives/CRK2LKYHW/p1680872840905409 |
@raulk We are parsing it here. There is still 2 solutions to this problem. The first one I already explained. The second one is to make it a cross call contract. EDIT: I also submitted in the FIP forum a proposition to add status field which would properly inform about the activation status of the deal. No new call just instead of throwing an error return the status. |
@rllola we might be talking past each other. The correct solution is for this library to not force a revert, but rather relay the exit code to the user via a return parameter, and let the user handle how they wish. Generally speaking, exit codes convey all kinds of important information that this library is currently dismissing. The concrete example case here is just one example of many where this unconditional low-level revert will be problematic and insufficient. |
Can we please build a PoC? |
Good, but the library still reverts unconditionally, making the exit code useless for the contract that's actually using the library. |
getDealActivation(dealID)
will revert the contract if the deal is expired.This is the comment above the function:
By this, I'm assuming that I can get a return value when the deal expired, including that scenario when the start epoch past before the sealing was done.
This is the return type of the function:
I don't know how this function would signal if the deal expired. If it is not the intended behavior that it is signaling that the deal expired, then I don't know how I should check first whether the deal is expired or not, so the smart contract call does not fail.
🔗 zboto Link
The text was updated successfully, but these errors were encountered: