Skip to content

Conversation

@njlie
Copy link
Contributor

@njlie njlie commented Dec 9, 2025

Changes proposed in this pull request

  • Adds a handlePartialPayment service call to the incoming payment service, to be called when encrypted data is transmitted through an ILP stream.

Context

Fixes RAF-1183 and fixes RAF-1184.

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Make sure that all checks pass
  • Bruno collection updated (if necessary)
  • Documentation issue created with user-docs label (if necessary)
  • OpenAPI specs updated (if necessary)

@github-actions github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. type: source Changes business logic labels Dec 9, 2025
@github-actions
Copy link

github-actions bot commented Dec 9, 2025

🚀 Performance Test Results

Test Configuration:

  • VUs: 4
  • Duration: 1m0s

Test Metrics:

  • Requests/s: 43.39
  • Iterations/s: 14.47
  • Failed Requests: 0.00% (0 of 2609)
📜 Logs

> [email protected] run-tests:testenv /home/runner/work/rafiki/rafiki/test/performance
> ./scripts/run-tests.sh -e test "-k" "-q" "--vus" "4" "--duration" "1m"

Cloud Nine GraphQL API is up: http://localhost:3101/graphql
Cloud Nine Wallet Address is up: http://localhost:3100/
Happy Life Bank Address is up: http://localhost:4100/
cloud-nine-wallet-test-backend already set
cloud-nine-wallet-test-auth already set
happy-life-bank-test-backend already set
happy-life-bank-test-auth already set
     data_received..................: 942 kB 16 kB/s
     data_sent......................: 2.0 MB 33 kB/s
     http_req_blocked...............: avg=8.05µs   min=2.53µs   med=5.3µs    max=1.03ms   p(90)=6.57µs   p(95)=7.19µs  
     http_req_connecting............: avg=791ns    min=0s       med=0s       max=993.42µs p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=91.52ms  min=6.46ms   med=75.97ms  max=546.81ms p(90)=156.28ms p(95)=182.21ms
       { expected_response:true }...: avg=91.52ms  min=6.46ms   med=75.97ms  max=546.81ms p(90)=156.28ms p(95)=182.21ms
     http_req_failed................: 0.00%  ✓ 0         ✗ 2609
     http_req_receiving.............: avg=88.71µs  min=24.88µs  med=78.26µs  max=2.34ms   p(90)=119.76µs p(95)=145.76µs
     http_req_sending...............: avg=38.77µs  min=11.3µs   med=27.85µs  max=1.98ms   p(90)=41.83µs  p(95)=57.43µs 
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=91.39ms  min=6.31ms   med=75.89ms  max=546.7ms  p(90)=156.11ms p(95)=182.11ms
     http_reqs......................: 2609   43.387428/s
     iteration_duration.............: avg=276.14ms min=185.59ms med=261.72ms max=1.08s    p(90)=338.17ms p(95)=373.85ms
     iterations.....................: 870    14.468019/s
     vus............................: 4      min=4       max=4 
     vus_max........................: 4      min=4       max=4 

@njlie njlie marked this pull request as ready for review December 12, 2025 18:44
Base automatically changed from nl/raf-1182 to main December 16, 2025 17:14
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.

Looks good, just a few comments

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
dataToTransmit: acc.name

Since we have it already anyway

incomingPaymentId: incomingPayment.id,
type: IncomingPaymentEventType.IncomingPaymentPartialPaymentReceived,
data: {
...incomingPayment.toData(0n),
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, we need to add a new property to the data that specifies the amount that was only sent in the "partial" payment, so the receiving ASE can make a decision whether to action it or not. The receivedAmount should remain unchanged, ie, it will include the cumulative amount received thus far for the incoming payment.

I will encapsulate this in a separate issue.

Comment on lines 607 to 608
Copy link
Contributor

Choose a reason for hiding this comment

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

If we keep the DB encryption secret as optional, this can just be: "Data to transmit to the recipient during the payment"

Comment on lines 3537 to 3543
Copy link
Contributor

Choose a reason for hiding this comment

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

I think instead of this, we can just assert that depositSpy was called with dataToTransmit (since the resolver doesn't really need to know about how it gets used in the method itself)

Comment on lines 771 to 773
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think the encryption should be an optional feature? I think this might be a good call, as it allows the integrator to decide whether they want it or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I figured leaving it open was the best possibility - I definitely can see cases where an integrator might not want it, and therefore not have to deal with another piece of the environment to configure.

@netlify
Copy link

netlify bot commented Jan 7, 2026

Deploy Preview for brilliant-pasca-3e80ec ready!

Name Link
🔨 Latest commit f797928
🔍 Latest deploy log https://app.netlify.com/projects/brilliant-pasca-3e80ec/deploys/695eaa84b9f964000800e40b
😎 Deploy Preview https://deploy-preview-3772--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 project configuration.

@njlie njlie force-pushed the nl/raf-1183/store-receiving-encrypted-data branch from 313abf0 to 97ada59 Compare January 7, 2026 18:47
@njlie njlie requested a review from mkurapov January 7, 2026 19:29
Copy link
Contributor

@sanducb sanducb 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!

@njlie njlie merged commit 5170538 into main Jan 9, 2026
38 of 58 checks passed
@njlie njlie deleted the nl/raf-1183/store-receiving-encrypted-data branch January 9, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: backend Changes in the backend package. pkg: frontend Changes in the frontend package. pkg: mock-account-service-lib pkg: mock-ase type: source Changes business logic type: tests Testing related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants