Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 78 additions & 8 deletions .github/workflows/openapi-breaking-changes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,20 @@ jobs:
--format markdown \
--fail-on ERR > breaking.md
status=$?
# The markdown format prints every finding with a :warning: marker, so it
# cannot be filtered by severity; singleline prefixes each with its level.
oasdiff breaking \
base/openapi.yaml head/openapi.yaml \
--format singleline \
--fail-on ERR > breaking.txt
singleline_status=$?
set -e
# Both runs diff the same specs with the same --fail-on, so their exit codes
# must agree; anything above 1 is a crash rather than a finding.
if [ "$status" -gt 1 ] || [ "$singleline_status" -ne "$status" ]; then
echo "::error::oasdiff exited with $status (markdown) / $singleline_status (singleline)"
exit 1
fi
echo "status=$status" >> "$GITHUB_OUTPUT"
if [ "$status" -ne 0 ] && [ -s breaking.md ]; then
echo "has_breaking=true" >> "$GITHUB_OUTPUT"
Expand All @@ -60,21 +74,77 @@ jobs:
fi

- name: Build comment body
id: body
if: steps.oasdiff.outputs.has_breaking == 'true'
env:
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
run: |
# GitHub rejects a comment body over 65536 characters, and one change to a
# shared schema yields a finding per inheriting subschema per endpoint.
MAX_ERRORS=100
MAX_WARNINGS=25
MAX_BODY=64000

errors=$(grep -c '^error ' breaking.txt || true)
warnings=$(grep -c '^warning ' breaking.txt || true)
# `error at head/openapi.yaml, in API GET /quotes added …` -> `- `GET /quotes` — added …`
fmt() { sed 's/^\(error\|warning\) at [^,]*, in API \([A-Z]* [^ ]*\) /- `\2` — /'; }

{
echo 'body<<COMMENT_EOF'
echo '## :warning: Breaking OpenAPI changes detected'
echo ''
echo 'This PR introduces breaking changes to `openapi.yaml`:'
echo "\`oasdiff\` reports **$errors error / $warnings warning** changes to \`openapi.yaml\`."
echo 'This PR will need approval from an API reviewer before merge.'
echo ''
echo "### Errors ($errors)"
echo ''
cat breaking.md
grep -m "$MAX_ERRORS" '^error ' breaking.txt | fmt
if [ "$errors" -gt "$MAX_ERRORS" ]; then
echo "- _…and $((errors - MAX_ERRORS)) more errors._"
fi
if [ "$warnings" -gt 0 ]; then
echo ''
echo "### Warnings ($warnings)"
echo ''
echo '<details><summary>Show sample</summary>'
echo ''
grep -m "$MAX_WARNINGS" '^warning ' breaking.txt | fmt
if [ "$warnings" -gt "$MAX_WARNINGS" ]; then
echo "- _…and $((warnings - MAX_WARNINGS)) more warnings._"
fi
echo ''
echo '</details>'
fi
echo ''
echo '---'
echo '_Detected by [oasdiff](https://github.com/oasdiff/oasdiff). This PR will need approval from an API reviewer before merge._'
echo 'COMMENT_EOF'
} >> "$GITHUB_OUTPUT"
echo "_Detected by [oasdiff](https://github.com/oasdiff/oasdiff). Full report: [job summary]($RUN_URL) or the \`oasdiff-report\` artifact._"
} > comment.md

if [ "$(wc -c < comment.md)" -gt "$MAX_BODY" ]; then
# Trim on line boundaries so a multi-byte character is never split.
LC_ALL=C awk -v max="$MAX_BODY" '{ n += length($0) + 1; if (n > max) exit } 1' \
comment.md > comment.trimmed
printf '\n_Comment truncated. Full report: [job summary](%s)._\n' "$RUN_URL" >> comment.trimmed
mv comment.trimmed comment.md
fi

- name: Publish full report to job summary
if: steps.oasdiff.outputs.has_breaking == 'true'
run: |
# The step summary is capped at 1 MiB.
{
echo '## oasdiff breaking changes'
echo ''
head -c 900000 breaking.md
} >> "$GITHUB_STEP_SUMMARY"

- name: Upload full report
if: steps.oasdiff.outputs.has_breaking == 'true'
uses: actions/upload-artifact@v4
with:
name: oasdiff-report
path: |
breaking.md
breaking.txt

- name: Ensure breaking-change label exists
if: steps.oasdiff.outputs.has_breaking == 'true'
Expand Down Expand Up @@ -108,7 +178,7 @@ jobs:
uses: marocchino/sticky-pull-request-comment@v2
with:
header: oasdiff-breaking-changes
message: ${{ steps.body.outputs.body }}
path: comment.md

- name: Clear PR comment (no breaking)
if: steps.oasdiff.outputs.has_breaking == 'false'
Expand Down
33 changes: 33 additions & 0 deletions mintlify/openapi.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 33 additions & 0 deletions openapi.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions openapi/components/schemas/common/PaymentAccountType.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@ enum:
- BASE_WALLET
- ETHEREUM_WALLET
- EMBEDDED_WALLET
- BITCOIN_L1
description: Type of payment account or wallet
example: USD_ACCOUNT
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
title: Bitcoin L1 Deposit Address
Comment thread
peterrojs marked this conversation as resolved.
allOf:
- $ref: ./BasePaymentAccountInfo.yaml
- type: object
required:
- address
properties:
accountType:
type: string
enum:
- BITCOIN_L1
address:
type: string
description: On-chain Bitcoin (L1) deposit address to send funds to
example: bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq
network:
type: string
description: The blockchain network for the deposit address.
enum:
- BITCOIN
example: BITCOIN
2 changes: 2 additions & 0 deletions openapi/components/schemas/common/PaymentInstructions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ properties:
- $ref: ../common/PaymentPolygonWalletInfo.yaml
- $ref: ../common/PaymentBaseWalletInfo.yaml
- $ref: ../common/PaymentEthereumWalletInfo.yaml
- $ref: ../common/PaymentBitcoinDepositAddressInfo.yaml
- $ref: ../common/PaymentEmbeddedWalletInfo.yaml
discriminator:
propertyName: accountType
Expand Down Expand Up @@ -87,6 +88,7 @@ properties:
POLYGON_WALLET: ../common/PaymentPolygonWalletInfo.yaml
BASE_WALLET: ../common/PaymentBaseWalletInfo.yaml
ETHEREUM_WALLET: ../common/PaymentEthereumWalletInfo.yaml
BITCOIN_L1: ../common/PaymentBitcoinDepositAddressInfo.yaml
AED_ACCOUNT: ../common/PaymentAedAccountInfo.yaml
KES_ACCOUNT: ../common/PaymentKesAccountInfo.yaml
MWK_ACCOUNT: ../common/PaymentMwkAccountInfo.yaml
Expand Down
6 changes: 6 additions & 0 deletions openapi/components/schemas/quotes/AccountDestination.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,10 @@ allOf:
The payment rail to use for the transfer. Must be one of the rails
supported by the destination account. If not specified, the system
will select a default rail.
refundAddress:
type: string
description: >-
Optional address the funds are returned to if the payout cannot be
completed. Applies to crypto off-ramp destinations.
example: bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq
Comment on lines +24 to +29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PR description is a little confusing - it says this is for offramps? For offramp refunds (e.g., USDB -> BTC offramp fails, so we want to refund) shouldn't that automatically just go back to the source, and would we ever not know that value?

I think it makes sense to me that this is not the case for onramps - if the flow fails I'm guessing you should be able to specify where your BTC should go back to if it doesn't work.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The confusing description is on me — I rewrote that section and mislabeled it. Fixing it now. And I think you're right on the substance; here's what I found digging into the server side.

Why the wording reads wrong. AccountDestination covers every account destination, including fiat bank accounts, where a refund address is meaningless. Calling it "off-ramp" made it sound like it applies to the fiat-payout case. It only has any meaning when the destination account is a crypto one (BTC out).

On whether it's needed at all — I think not, for the reason you gave. For a BTC-out payout, Grid is the party funding the upstream swap, so a failed payout refunds to a Grid-controlled address Grid already knows; the caller has nothing to contribute. And in the case where there genuinely is an inbound on-chain leg whose sender we can't infer (customer deposits BTC/stables to a generated address), that's already covered by RealtimeFundingQuoteSource.refundAddress — the source side, which is exactly the asymmetry you described.

Where the field probably came from. The upstream Orchestra API requires refundAddress on both POST /v1/orchestration/quote and POST /v1/orchestration/submit — in sparkcore/sparkcore/grid/flashnet/orchestra_client.py it's a required kwarg on both and always written into the body, for every order regardless of direction. It's an easy step from "the upstream call always needs one" to "we need one per direction," but those aren't the same thing: the upstream needing a value doesn't mean the API caller has to supply it.

Nothing consumes it yet. That flashnet package currently has no non-test importers, and there's no BITCOIN_L1 handling in sparkcore at all — consistent with the integration still being on stopgaps. So dropping the field costs nothing today.

The cost is asymmetric, which is what tips it for me: removing it now is free, and adding it back later is additive and non-breaking. Shipping it and removing it later is a breaking change. Given it's unused and the semantics are dubious, the cheap and safe move is to drop AccountDestination.refundAddress and keep the source-side one.

@peterrojs — you wrote the Orchestra client, so you'd know the flow better than I can infer it: is there an off-ramp shape where the refund target isn't derivable from the source, e.g. the customer funds the swap directly rather than out of a Grid-held balance? If not, say the word and I'll drop the field and rebundle. I didn't want to remove something you added deliberately for work you're mid-way through wiring up.


🤖 resonant-reactor-4(#4) | Feedback

description: Destination account details
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ allOf:
on, so the correct deposit address can be generated. Example values:
`SOLANA`, `ETHEREUM`, `BASE`, `POLYGON`, `SPARK`, `LIGHTNING`, `BITCOIN`.
example: SOLANA
refundAddress:
type: string
description: >-
Optional address the deposited funds are returned to if the crypto
deposit cannot be routed to complete the payment. Applies to on-chain
crypto funding sources.
example: bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq
description: >-
Fund the quote using a real-time funding source (RTP, SEPA Instant, Spark, Stables, etc.).
This will require manual just-in-time funding using `paymentInstructions` in the response.
Expand Down
Loading