Skip to content

Escrow Cancellation#3357

Closed
flopez7 wants to merge 23 commits intodevelopfrom
feat/escrow-cancellation
Closed

Escrow Cancellation#3357
flopez7 wants to merge 23 commits intodevelopfrom
feat/escrow-cancellation

Conversation

@flopez7
Copy link
Copy Markdown
Contributor

@flopez7 flopez7 commented May 20, 2025

Issue tracking

#3287

Context behind the change

  • Added fund reservation in escrow contract and CancellationRefund and CancellationRequested events to track cancellation process and refunds.
  • Updated Subgraph to handle new events from escrow contract
  • Updated SDKs to add the new var needed for fund reservation and some new methods for querying CancellationRefund and CancellationRequested events
  • Created new webhook from JL to RecO for escrow cancellation and updated cancellation logic. Now we have to check CancellationRefundEvents for getting the amount we have to refund to the user instead of receiving it directly from the SDK cancel method.
  • In RecO, added new fund reservation logic and new method for handling escrow cancellation by calling StoreResults and sending the results to RepO
  • Updated Rep Oracle logic to properly handle cancelled escrows, calculate payouts using reserved funds and send webhooks back to job launcher and exchange oracle.

How has this been tested?

Deployed locally and completed the fortune flow with some escrows. Tested normal and cancellation flow
Run unit tests

Release plan

Important: Check that there are no active escrows before deployment to avoid errors due to StoreResult contract change. Also we need to make sure CVAT and Audino are updated to use new contract version

  • Deploy smart contracts
  • Deploy the new SDK version
  • Deploy new Subgraphs

Potential risks; What to monitor; Reverse plan

Since the core has been updated, we have to check each step to make sure it works correctly.

@flopez7 flopez7 requested a review from portuu3 May 20, 2025 13:38
@flopez7 flopez7 self-assigned this May 20, 2025
@vercel
Copy link
Copy Markdown

vercel bot commented May 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
human-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2025 2:42pm
human-dashboard-frontend ❌ Failed (Inspect) Jul 23, 2025 2:42pm
staking-dashboard ❌ Failed (Inspect) Jul 23, 2025 2:42pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
faucet-frontend ⬜️ Ignored (Inspect) Visit Preview Jul 23, 2025 2:42pm
faucet-server ⬜️ Ignored (Inspect) Visit Preview Jul 23, 2025 2:42pm

@flopez7 flopez7 marked this pull request as draft May 20, 2025 13:38
Copy link
Copy Markdown
Collaborator

@portuu3 portuu3 left a comment

Choose a reason for hiding this comment

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

For bulkPayout we should have the changes from https://github.com/humanprotocol/human-protocol/pull/3261/files. Merge it first

Comment thread packages/core/contracts/Escrow.sol Outdated
Comment thread packages/core/contracts/Escrow.sol Outdated
Comment thread packages/core/contracts/Escrow.sol Outdated
@portuu3
Copy link
Copy Markdown
Collaborator

portuu3 commented May 20, 2025

For bulkPayout we should have the changes from https://github.com/humanprotocol/human-protocol/pull/3261/files. Merge it first

* Fix possible no assignments error when there are

* Update readme

* Update cvat sdk versions

* Address api changes

* Improve performance of some cvat calls

* Improve description
* Draw roi point along with bbox in skeleton tasks

* Relax filtering for overlapping gt skeletons (#3358)
@flopez7 flopez7 force-pushed the feat/escrow-cancellation branch from 3f4dce8 to 941d544 Compare May 21, 2025 09:19
@vercel vercel bot temporarily deployed to Preview – staking-dashboard May 21, 2025 09:21 Inactive
@vercel vercel bot temporarily deployed to Preview – human-dashboard-frontend May 21, 2025 09:21 Inactive
@vercel vercel bot temporarily deployed to Preview – human-app May 21, 2025 09:21 Inactive
* fix payouts when fee calculation is truncated

* fix tests and use faker in the root of the repository

* document escrow contract and improve tests

* Delete unnecessary check for BULK_MAX_VALUE

* remove unused modifier

* Undo faker package changes for updating yarn

---------

Co-authored-by: Francisco López <francislopez977@gmail.com>
Comment thread packages/core/contracts/Escrow.sol Outdated
Comment thread packages/core/contracts/Escrow.sol Outdated
Comment thread packages/core/contracts/Escrow.sol Outdated
…avoid remaining funds and update tests for dynamic amounts
await this.jobService.resumeJob(webhook);
break;

case EventType.ESCROW_CANCELED:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we can delete this event type, we are not using it

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.

I added it to avoid returning error in case ESCROW_CANCELED type is received.

public async processEscrowCancellation(
jobEntity: JobEntity,
): Promise<EscrowCancelDto> {
public async processEscrowCancellation(jobEntity: JobEntity): Promise<void> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

return is unnecessary here

Comment thread packages/apps/job-launcher/server/src/modules/job/job.service.ts Outdated
Comment thread packages/core/contracts/Escrow.sol Outdated
status == EscrowStatuses.ToCancel,
'Escrow not in Pending, Partial or ToCancel status state'
);
if (_fundsToReserve != 0) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why only url and hash are required if _fundsToReserve != 0, in any case I think it should _fundsToReserve > 0

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.

Since _fundsToReserve is a uint256, both conditions (!= 0 and > 0) are equivalent.
I used this condition to allow canceling escrows when no solutions are received; in those cases, we don't have any URL or hash, so we must allow those fields to be empty.
That's why the check is only enforced when _fundsToReserve is not zero.

@vercel vercel bot temporarily deployed to Preview – human-dashboard-frontend June 19, 2025 16:20 Inactive
@vercel vercel bot temporarily deployed to Preview – human-app June 19, 2025 16:20 Inactive
@vercel vercel bot temporarily deployed to Preview – staking-dashboard June 19, 2025 16:20 Inactive
@vercel vercel bot temporarily deployed to Preview – human-app June 23, 2025 11:00 Inactive
@vercel vercel bot temporarily deployed to Preview – human-dashboard-frontend June 23, 2025 11:00 Inactive
@vercel vercel bot temporarily deployed to Preview – staking-dashboard June 23, 2025 11:00 Inactive
@portuu3
Copy link
Copy Markdown
Collaborator

portuu3 commented Aug 29, 2025

Changes applied in #3482

@portuu3 portuu3 closed this Aug 29, 2025
@portuu3 portuu3 deleted the feat/escrow-cancellation branch August 29, 2025 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Contracts Core documentation Improvements or additions to documentation Fortune Fortune app

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants