Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(2662)!: 2phase incoming and outgoing payments #2671

Merged
merged 27 commits into from May 15, 2024
Merged

feat(2662)!: 2phase incoming and outgoing payments #2671

merged 27 commits into from May 15, 2024

Conversation

koekiebox
Copy link
Collaborator

@koekiebox koekiebox commented Apr 22, 2024 •
edited by mkurapov

Changes proposed in this pull request

Context

Fixes #2662

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass
  • Bruno collection updated

@github-actions github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. pkg: frontend Changes in the frontend package. type: source Changes business logic pkg: mock-ase pkg: mock-account-service-lib labels Apr 22, 2024
@netlify Netlify
Copy link

netlify bot commented Apr 22, 2024 •
edited

✅ Deploy Preview for brilliant-pasca-3e80ec ready!

Name Link
🔨 Latest commit 0173ad6
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/6644c80ebaf89a00080637ce
😎 Deploy Preview https://deploy-preview-2671--brilliant-pasca-3e80ec.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@koekiebox koekiebox self-assigned this Apr 22, 2024
@koekiebox koekiebox mentioned this pull request Apr 22, 2024
Closed
2 tasks
@github-actions github-actions bot added pkg: documentation Changes in the documentation package. type: documentation Improvements or additions to documentation labels Apr 23, 2024
@github-actions github-actions bot added the type: localenv Local playground label Apr 29, 2024
@sabineschaller
Copy link
Member

@koekiebox we discussed this last week and we were wondering if we could make the 2-phase transfer optional on all withdrawals. What do you think?

@koekiebox
Copy link
Collaborator Author

@koekiebox we discussed this last week and we were wondering if we could make the 2-phase transfer optional on all withdrawals. What do you think?

@sabineschaller this should not be a challenge. The accountingService.createWithdrawal will create a 1 or 2 Phase transfer depending on the timeout parameter being > 0. So we can update each of the withdrawal mutations to also have a timeout field as part of the input? Which will also mean the caller would be able to control the timeout for a 2-phase transfer. What do you think?

@sabineschaller
Copy link
Member

@sabineschaller this should not be a challenge. The accountingService.createWithdrawal will create a 1 or 2 Phase transfer depending on the timeout parameter being > 0. So we can update each of the withdrawal mutations to also have a timeout field as part of the input? Which will also mean the caller would be able to control the timeout for a 2-phase transfer. What do you think?

@koekiebox Yes, that is exactly how I thought this would work 🚀

@koekiebox
Merge branch 'refs/heads/main' into issues/2662-two-phase
dcb7192 
# Conflicts:
#	localenv/mock-account-servicing-entity/generated/graphql.ts
#	packages/backend/src/graphql/generated/graphql.schema.json
#	packages/backend/src/graphql/generated/graphql.ts
#	packages/frontend/app/generated/graphql.ts
#	packages/mock-account-service-lib/src/generated/graphql.ts
#	test/integration/lib/generated/graphql.ts
@koekiebox
feat(2662): introduce the timeout as optional argument for withdraws.
40ca768
@koekiebox
feat(2662): fix test cases.
0fe676d
@koekiebox
feat(2662): fix front-end issues wgt timeout.
6d78472
@koekiebox
feat(2662): bruno updates.
4801a8c
@koekiebox
Merge branch 'refs/heads/main' into issues/2662-two-phase
6f12ee8
@koekiebox
feat(2662): doc updates.
e892eb7
@koekiebox
feat(2662): doc updates.
96a4f81
@koekiebox
feat(2662): liq test case updates.
ffb1c3a
@koekiebox koekiebox marked this pull request as ready for review May 6, 2024 10:25
@koekiebox koekiebox requested a review from BlairCurrey May 6, 2024 10:53
BlairCurrey
BlairCurrey previously approved these changes May 14, 2024
View reviewed changes
mkurapov
mkurapov previously approved these changes May 14, 2024
View reviewed changes
@koekiebox koekiebox dismissed stale reviews from mkurapov and BlairCurrey via cd92203 May 15, 2024 06:18
mkurapov
mkurapov reviewed May 15, 2024
View reviewed changes
Copy link
Contributor

@mkurapov mkurapov left a comment

Choose a reason for hiding this comment

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

Looking good, just minor formatting

packages/documentation/src/content/docs/integration/event-handlers.mdx Outdated Show resolved Hide resolved
packages/documentation/src/content/docs/integration/event-handlers.mdx Outdated Show resolved Hide resolved
@koekiebox koekiebox requested a review from mkurapov May 15, 2024 08:56
BlairCurrey
BlairCurrey previously approved these changes May 15, 2024
View reviewed changes
@koekiebox koekiebox merged commit 946fb7c into main May 15, 2024
42 checks passed
@koekiebox koekiebox deleted the issues/2662-two-phase branch May 15, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkurapov mkurapov mkurapov approved these changes

@JoblersTune JoblersTune Awaiting requested review from JoblersTune

@BlairCurrey BlairCurrey Awaiting requested review from BlairCurrey

Assignees

@koekiebox koekiebox

Labels
breaking Issue/PR that introduces breaking changes pkg: backend Changes in the backend package. pkg: documentation Changes in the documentation package. pkg: frontend Changes in the frontend package. pkg: mock-account-service-lib pkg: mock-ase type: documentation Improvements or additions to documentation type: localenv Local playground type: source Changes business logic type: tests Testing related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make every withdrawal an optional 2 phase transfer
5 participants
@koekiebox @sabineschaller @BlairCurrey @mkurapov @JoblersTune