Exported API for market deal activation state #819

anorth · 2022-11-09T02:55:40Z

A basic API for deal state. This is far from an ideal state API, but works with what the market actor actually knows. There's no information preserved after a deal is cleaned up.

Remove duplication from Built-in market API for deal proposal metadata #818 before merging

codecov-commenter · 2022-11-09T03:19:28Z

Codecov Report

❗ No coverage uploaded for pull request base (integration/builtin-api@ad4f69b). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files

@@                    Coverage Diff                     @@
##             integration/builtin-api     #819   +/-   ##
==========================================================
  Coverage                           ?   79.35%           
==========================================================
  Files                              ?       95           
  Lines                              ?    21162           
  Branches                           ?        0           
==========================================================
  Hits                               ?    16794           
  Misses                             ?     4368           
  Partials                           ?        0

arajasek

Yeah, this leaves something to desire. I think there's 3 cases we can distinguish:

this deal never existed, USR_NOT_FOUND is a meaningful return here.
this deal's state was found, Activation and Termination seem like good epochs to return.
this deal has been fully GCed (including its proposal). USR_NOT_FOUND isn't the best return here, I'd like us to actually indicate this as a successful return, since it is meaningful (albeit incomplete) info about the deal.

arajasek · 2022-11-09T15:37:13Z

actors/market/src/lib.rs

+            }),
+            None => {
+                // With no state, the deal may have been published but not activated.
+                // If there's no proposal, we can't distinguish between never existing,


So, we can be a little more clever than this. We can readily detect the "never existing" case -- it's simply params.id >= st.next_id.

I agree that "failed to activate" vs. "cleaned up after completion/termination" are indistinguishable, though.

actors/market/src/lib.rs

arajasek

LGTM, some nits.

arajasek · 2022-11-14T23:45:09Z

actors/market/src/lib.rs

@@ -60,6 +60,9 @@ fil_actors_runtime::wasm_trampoline!(Actor);

 pub const NO_ALLOCATION_ID: u64 = 0;

+// An exit code indicating that information about a past deal is no longer available.
+pub const EX_DEAL_EXPIRED: ExitCode = ExitCode::new(32);


Should we not have a FIRST_ACTOR_SPECIFIC_EXIT_CODE somewhere, and define this in terms of that?

Probably, but will defer to #841 (this isn't the first)

actors/market/src/lib.rs

actors/market/src/state.rs

* Remove the market actor state mutation pattern (#734) Fixes #656 * Proof of concept exported API for Account actor (#797) * Export stable methods for public access (#807) * Export Datacap Actor methods * Export Init Actor methods * Export Market Actor methods * Export Miner Actor methods * Export Multisig Actor methods * Export Verifreg Actor methods * Address review * Restrict internal APIs of all actors (#809) * Exported API method for market actor escrow/locked balance (#812) * Power actor: Add exported getters for raw power (#810) * Power actor: Add exported getters for raw power * FRC-XXXX is FRC-0042 * Power actor: network_raw_power: Return this_epoch_raw_byte_power * Power actor: miner_raw_power: Return whether above consensus min power * Power actor: types: serialize one-element structs transparently * Address review * Miner actor: Add exported getters for info and monies (#811) * Miner actor: Add exported getters for info and monies * Tweak comment * Miner actor: Replace GetWorker and GetControls with IsControllingAddress * Miner actor: Add exported GetAvailableBalance * Miner actor: Add exported GetVestingFunds * Miner actor: Remove exported monies getters * Miner actor: types: serialize one-element structs transparently * Address review * Address review * Built-in market API for deal proposal metadata (#818) * Call exported authenticate method from PSD (#829) Co-authored-by: zenground0 <[email protected]> * Drop CALLER_TYPES_SIGNABLE and signable caller validation (#821) * Market actor: Minor improvements to two exported getters (#826) * Market actor: GetDealTermExported: Return (start_epoch, duration) * Market actor: Export getter for deal total price * Exported API for market deal activation state (#819) * Paych actor: Drop account req, use AuthenticateMessage to verify sigs (#824) * Paych actor: Drop account req, use AuthenticateMessage to verify sigs * Address review * Address review * Account actor: Deprecate AuthenticateMessage (#856) * Market actor: Export PublishStorageDeals (#857) * Miner: Export several more methods (#863) * Miner: Export ChangeWorkerAddress * Miner: Export ChangePeerID * Miner: Export WithdrawBalance * Miner: Export ChangeMultiaddrs * Miner: Export ConfirmUpdateWorkerKey * Miner: Export RepayDebt * Miner: Export ChangeOwnerAddress * Miner: Add exported getters for PeerID & multiaddrs * Miner: Refactor: Rename ConfirmUpdateWorkerKey to ConfirmChangeWorkerAddress * Power actor: Export methods to CreateMiner and get miner counts (#868) * Power: Export CreateMiner * Power Actor: Export MinerCount and MinerConsensusCount * Update actors/power/src/lib.rs Co-authored-by: Alex <[email protected]> Co-authored-by: Alex <[email protected]> * Verifreg: Export AddVerifiedClient and GetClaims (#873) * Verifreg: Rename AddVerifierClientParams to AddVerifiedClientParams * Verifreg: Export AddVerifiedClient and GetClaims * Datacap actor: Modify exported methods (#909) * Datacap: Export Mint and Destroy * Datacap actor: Deprecate all internal methods * Datacap actor: Rename BalanceOf to Balance * Datacap actor: Add Granularity method * fix: comments on newly exported methods (#910) Co-authored-by: RK <[email protected]> Co-authored-by: Alex <[email protected]> Co-authored-by: ZenGround0 <[email protected]> Co-authored-by: zenground0 <[email protected]>

* Proof of concept exported API for Account actor (#797) * Export stable methods for public access (#807) * Export Datacap Actor methods * Export Init Actor methods * Export Market Actor methods * Export Miner Actor methods * Export Multisig Actor methods * Export Verifreg Actor methods * Address review * Restrict internal APIs of all actors (#809) * Exported API method for market actor escrow/locked balance (#812) * Power actor: Add exported getters for raw power (#810) * Power actor: Add exported getters for raw power * FRC-XXXX is FRC-0042 * Power actor: network_raw_power: Return this_epoch_raw_byte_power * Power actor: miner_raw_power: Return whether above consensus min power * Power actor: types: serialize one-element structs transparently * Address review * Miner actor: Add exported getters for info and monies (#811) * Miner actor: Add exported getters for info and monies * Tweak comment * Miner actor: Replace GetWorker and GetControls with IsControllingAddress * Miner actor: Add exported GetAvailableBalance * Miner actor: Add exported GetVestingFunds * Miner actor: Remove exported monies getters * Miner actor: types: serialize one-element structs transparently * Address review * Address review * Built-in market API for deal proposal metadata (#818) * Call exported authenticate method from PSD (#829) Co-authored-by: zenground0 <[email protected]> * Drop CALLER_TYPES_SIGNABLE and signable caller validation (#821) * Market actor: Minor improvements to two exported getters (#826) * Market actor: GetDealTermExported: Return (start_epoch, duration) * Market actor: Export getter for deal total price * Exported API for market deal activation state (#819) * Paych actor: Drop account req, use AuthenticateMessage to verify sigs (#824) * Paych actor: Drop account req, use AuthenticateMessage to verify sigs * Address review * Address review * Account actor: Deprecate AuthenticateMessage (#856) * Market actor: Export PublishStorageDeals (#857) * Miner: Export several more methods (#863) * Miner: Export ChangeWorkerAddress * Miner: Export ChangePeerID * Miner: Export WithdrawBalance * Miner: Export ChangeMultiaddrs * Miner: Export ConfirmUpdateWorkerKey * Miner: Export RepayDebt * Miner: Export ChangeOwnerAddress * Miner: Add exported getters for PeerID & multiaddrs * Miner: Refactor: Rename ConfirmUpdateWorkerKey to ConfirmChangeWorkerAddress * Power actor: Export methods to CreateMiner and get miner counts (#868) * Power: Export CreateMiner * Power Actor: Export MinerCount and MinerConsensusCount * Update actors/power/src/lib.rs Co-authored-by: Alex <[email protected]> Co-authored-by: Alex <[email protected]> * Verifreg: Export AddVerifiedClient and GetClaims (#873) * Verifreg: Rename AddVerifierClientParams to AddVerifiedClientParams * Verifreg: Export AddVerifiedClient and GetClaims * Datacap actor: Modify exported methods (#909) * Datacap: Export Mint and Destroy * Datacap actor: Deprecate all internal methods * Datacap actor: Rename BalanceOf to Balance * Datacap actor: Add Granularity method * fix: comments on newly exported methods (#910) * Miner: Export method to GetPendingOwner * MarketNotifyDeal (#944) Co-authored-by: zenground0 <[email protected]> * Verifreg: Call AuthenticateMessage instead of validating siggys * Multisig: Do not export any functionality (#967) * use const for EX_DEAL_EXPIRED (#954) * Address review Co-authored-by: Alex <[email protected]> Co-authored-by: ZenGround0 <[email protected]> Co-authored-by: zenground0 <[email protected]> Co-authored-by: Shashank <[email protected]>

…ecoin-project#1004) * Proof of concept exported API for Account actor (filecoin-project#797) * Export stable methods for public access (filecoin-project#807) * Export Datacap Actor methods * Export Init Actor methods * Export Market Actor methods * Export Miner Actor methods * Export Multisig Actor methods * Export Verifreg Actor methods * Address review * Restrict internal APIs of all actors (filecoin-project#809) * Exported API method for market actor escrow/locked balance (filecoin-project#812) * Power actor: Add exported getters for raw power (filecoin-project#810) * Power actor: Add exported getters for raw power * FRC-XXXX is FRC-0042 * Power actor: network_raw_power: Return this_epoch_raw_byte_power * Power actor: miner_raw_power: Return whether above consensus min power * Power actor: types: serialize one-element structs transparently * Address review * Miner actor: Add exported getters for info and monies (filecoin-project#811) * Miner actor: Add exported getters for info and monies * Tweak comment * Miner actor: Replace GetWorker and GetControls with IsControllingAddress * Miner actor: Add exported GetAvailableBalance * Miner actor: Add exported GetVestingFunds * Miner actor: Remove exported monies getters * Miner actor: types: serialize one-element structs transparently * Address review * Address review * Built-in market API for deal proposal metadata (filecoin-project#818) * Call exported authenticate method from PSD (filecoin-project#829) Co-authored-by: zenground0 <[email protected]> * Drop CALLER_TYPES_SIGNABLE and signable caller validation (filecoin-project#821) * Market actor: Minor improvements to two exported getters (filecoin-project#826) * Market actor: GetDealTermExported: Return (start_epoch, duration) * Market actor: Export getter for deal total price * Exported API for market deal activation state (filecoin-project#819) * Paych actor: Drop account req, use AuthenticateMessage to verify sigs (filecoin-project#824) * Paych actor: Drop account req, use AuthenticateMessage to verify sigs * Address review * Address review * Account actor: Deprecate AuthenticateMessage (filecoin-project#856) * Market actor: Export PublishStorageDeals (filecoin-project#857) * Miner: Export several more methods (filecoin-project#863) * Miner: Export ChangeWorkerAddress * Miner: Export ChangePeerID * Miner: Export WithdrawBalance * Miner: Export ChangeMultiaddrs * Miner: Export ConfirmUpdateWorkerKey * Miner: Export RepayDebt * Miner: Export ChangeOwnerAddress * Miner: Add exported getters for PeerID & multiaddrs * Miner: Refactor: Rename ConfirmUpdateWorkerKey to ConfirmChangeWorkerAddress * Power actor: Export methods to CreateMiner and get miner counts (filecoin-project#868) * Power: Export CreateMiner * Power Actor: Export MinerCount and MinerConsensusCount * Update actors/power/src/lib.rs Co-authored-by: Alex <[email protected]> Co-authored-by: Alex <[email protected]> * Verifreg: Export AddVerifiedClient and GetClaims (filecoin-project#873) * Verifreg: Rename AddVerifierClientParams to AddVerifiedClientParams * Verifreg: Export AddVerifiedClient and GetClaims * Datacap actor: Modify exported methods (filecoin-project#909) * Datacap: Export Mint and Destroy * Datacap actor: Deprecate all internal methods * Datacap actor: Rename BalanceOf to Balance * Datacap actor: Add Granularity method * fix: comments on newly exported methods (filecoin-project#910) * Miner: Export method to GetPendingOwner * MarketNotifyDeal (filecoin-project#944) Co-authored-by: zenground0 <[email protected]> * Verifreg: Call AuthenticateMessage instead of validating siggys * Multisig: Do not export any functionality (filecoin-project#967) * use const for EX_DEAL_EXPIRED (filecoin-project#954) * Address review Co-authored-by: Alex <[email protected]> Co-authored-by: ZenGround0 <[email protected]> Co-authored-by: zenground0 <[email protected]> Co-authored-by: Shashank <[email protected]>

anorth requested a review from arajasek November 9, 2022 02:55

arajasek reviewed Nov 9, 2022

View reviewed changes

anorth force-pushed the anorth/market-api-state branch from d0ea2aa to 24ad173 Compare November 9, 2022 21:40

anorth requested a review from arajasek November 10, 2022 07:47

anorth mentioned this pull request Nov 10, 2022

Change market deal metadata methods to return EX_DEAL_EXPIRED if the deal ID is used #830

Closed

arajasek approved these changes Nov 14, 2022

View reviewed changes

anorth added 3 commits November 15, 2022 14:55

Exported API for market deal activation state

1fe4543

New exit code for expired deals

67107eb

review

383f114

anorth force-pushed the anorth/market-api-state branch from 73c7ae0 to 383f114 Compare November 15, 2022 02:04

anorth enabled auto-merge (squash) November 15, 2022 02:08

anorth merged commit 54c278f into integration/builtin-api Nov 15, 2022

anorth deleted the anorth/market-api-state branch November 15, 2022 02:22

arajasek pushed a commit that referenced this pull request Nov 16, 2022

Exported API for market deal activation state (#819)

3bcd854

arajasek pushed a commit that referenced this pull request Nov 16, 2022

Exported API for market deal activation state (#819)

6b216fc

arajasek pushed a commit that referenced this pull request Nov 17, 2022

Exported API for market deal activation state (#819)

607a2b5

arajasek pushed a commit that referenced this pull request Nov 17, 2022

Exported API for market deal activation state (#819)

f4496a2

arajasek pushed a commit that referenced this pull request Nov 17, 2022

Exported API for market deal activation state (#819)

7778d8c

arajasek pushed a commit that referenced this pull request Nov 21, 2022

Exported API for market deal activation state (#819)

d92b48a

arajasek pushed a commit that referenced this pull request Nov 23, 2022

Exported API for market deal activation state (#819)

776fa8a

arajasek pushed a commit that referenced this pull request Nov 27, 2022

Exported API for market deal activation state (#819)

8501cbf

arajasek pushed a commit that referenced this pull request Nov 28, 2022

Exported API for market deal activation state (#819)

440a9e8

arajasek pushed a commit that referenced this pull request Nov 28, 2022

Exported API for market deal activation state (#819)

7b66945

arajasek pushed a commit that referenced this pull request Nov 29, 2022

Exported API for market deal activation state (#819)

803a6e1

arajasek pushed a commit that referenced this pull request Nov 30, 2022

Exported API for market deal activation state (#819)

76ac0a5

arajasek pushed a commit that referenced this pull request Nov 30, 2022

Exported API for market deal activation state (#819)

4673ef2

arajasek pushed a commit that referenced this pull request Dec 5, 2022

Exported API for market deal activation state (#819)

ece6524

arajasek pushed a commit that referenced this pull request Dec 7, 2022

Exported API for market deal activation state (#819)

969f086

arajasek pushed a commit that referenced this pull request Dec 11, 2022

Exported API for market deal activation state (#819)

b26fc74

shamb0 pushed a commit to shamb0/builtin-actors that referenced this pull request Jan 31, 2023

Exported API for market deal activation state (filecoin-project#819)

b0d8827

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exported API for market deal activation state #819

Exported API for market deal activation state #819

anorth commented Nov 9, 2022 •

edited

Loading

codecov-commenter commented Nov 9, 2022 •

edited

Loading

arajasek left a comment

arajasek Nov 9, 2022

arajasek left a comment

arajasek Nov 14, 2022

anorth Nov 15, 2022 •

edited

Loading

Exported API for market deal activation state #819

Exported API for market deal activation state #819

Conversation

anorth commented Nov 9, 2022 • edited Loading

codecov-commenter commented Nov 9, 2022 • edited Loading

Codecov Report

arajasek left a comment

Choose a reason for hiding this comment

arajasek Nov 9, 2022

Choose a reason for hiding this comment

arajasek left a comment

Choose a reason for hiding this comment

arajasek Nov 14, 2022

Choose a reason for hiding this comment

anorth Nov 15, 2022 • edited Loading

Choose a reason for hiding this comment

anorth commented Nov 9, 2022 •

edited

Loading

codecov-commenter commented Nov 9, 2022 •

edited

Loading

anorth Nov 15, 2022 •

edited

Loading