Skip to content

Escrow contract integration #1910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open

Conversation

mariacarmina
Copy link
Member

@mariacarmina mariacarmina commented Feb 3, 2025

Fixes #1900 .

Changes proposed in this PR:

  • Created Escrow contract class
  • tests

@mariacarmina mariacarmina self-assigned this Feb 3, 2025
@mariacarmina mariacarmina changed the title Created Escrow contract class. Escrow contract integration Feb 3, 2025
@mariacarmina mariacarmina marked this pull request as ready for review February 21, 2025 21:19
@alexcos20 alexcos20 changed the base branch from main to release_4.0 March 4, 2025 09:29
- name: Run Ganache with Barge
working-directory: ${{ github.workspace }}/barge
run: |
export CONTRACTS_VERSION=escrow
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export CONTRACTS_VERSION=escrow
env:
CONTRACTS_VERSION: escrow

Copy link
Contributor

Choose a reason for hiding this comment

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

we should also check the conflicts

Copy link
Member

@bogdanfazakas bogdanfazakas left a comment

Choose a reason for hiding this comment

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

looks good but we need the things mentioned here this yml change and the conflicts issue

Base automatically changed from release_4.0 to main March 18, 2025 07:27
@giurgiur99 giurgiur99 self-requested a review March 26, 2025 08:07
@jamiehewitt15
Copy link
Member

Have you looked into the failing tests?

@giurgiur99
Copy link
Contributor

Have you looked into the failing tests?

I am looking over them now

Copy link
Member Author

@mariacarmina mariacarmina left a comment

Choose a reason for hiding this comment

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

Thank you @giurgiur99 for fixing this PR! The escrow version container for the contracts got updated with latest modifications from 2.3.0-next.3 and implicit the functions definitions changed. I suggest to not merge this yet because we have the contract version set to escrow and maybe we do not need to merge now escrow payments into ocean.js.

@mariacarmina mariacarmina force-pushed the escrow-contract-integration branch from 11dd61b to dda89da Compare April 11, 2025 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Escrow integration
5 participants