Skip to content

feat(binance): add spot OpenAPI skill and Ed25519 signer#187

Merged
jolestar merged 1 commit intomainfrom
feature/binance-spot-ed25519
Mar 9, 2026
Merged

feat(binance): add spot OpenAPI skill and Ed25519 signer#187
jolestar merged 1 commit intomainfrom
feature/binance-spot-ed25519

Conversation

@jolestar
Copy link
Collaborator

@jolestar jolestar commented Mar 9, 2026

What

  • add a curated Binance Spot OpenAPI wrapper skill with separate mainnet/testnet link flows
  • add ed25519_query_v1 alongside the existing HMAC query signer for Binance-style signed requests
  • fix OpenAPI auth resolution so bindings can match the final operation path and schema URL fetches are not polluted by business auth
  • document Spot testnet key acquisition, Ed25519 vs HMAC setup, and common Binance key mismatches

Why

Binance Spot testnet now practically needs Ed25519 support for the recommended key flow, and the Spot skill was missing the operational guidance needed to obtain and pair the correct API key with signing material.

How

  • extend auth signer config/runtime to support PKCS#8 PEM Ed25519 signing with base64 signatures
  • keep hmac_query_v1 unchanged for compatibility
  • add targeted unit and CLI tests for signer parsing and signing behavior
  • add the new skills/binance-spot-openapi-skill package and validation script
  • update the skill docs with testnet setup steps and the -1022 mismatch pitfall

Testing

  • bash skills/binance-spot-openapi-skill/scripts/validate.sh
  • cargo test auth::tests:: --lib -- --test-threads=1
  • cargo test --test auth_binding_cli_test -- --test-threads=1
  • cargo test daemon::tests:: --lib -- --test-threads=1
  • cargo test openapi::tests:: --lib -- --test-threads=1
  • live Binance Spot testnet verification for Ed25519:
    • get:/api/v3/account
    • post:/api/v3/order/test

Copilot AI review requested due to automatic review settings March 9, 2026 06:37
@holonbot
Copy link
Contributor

holonbot bot commented Mar 9, 2026

Review: PR #187 - feat(binance): add spot OpenAPI skill and Ed25519 signer

Summary

This PR adds Ed25519 query signing support alongside the existing HMAC SHA256 signer, introduces a curated Binance Spot OpenAPI skill, and fixes OpenAPI auth resolution to prevent business query parameters from polluting schema URL fetches. The implementation is well-tested with comprehensive unit tests, CLI integration tests, and skill validation scripts.

Overall Assessment: APPROVE with minor suggestions

Key Findings

Strengths

  1. Comprehensive Ed25519 Implementation

    • Proper PKCS#8 PEM parsing using ed25519-dalek with the pkcs8 and pem features
    • Base64 signature encoding as required by Binance
    • Extends the existing signer architecture cleanly with minimal duplication
  2. OpenAPI Auth Fix (src/adapters/openapi.rs:2013-2046)

    • The schema_requests_apply_auth() method correctly prevents business auth from polluting schema URL fetches when --schema-url is provided
    • Added test coverage confirms the fix works as intended
  3. Runtime Auth Resolution (src/daemon.rs:2640-2670)

    • The openapi_runtime_endpoint() helper correctly reconstructs the final operation URL for auth binding lookup
    • This enables path-prefix-level bindings to work properly with OpenAPI operations
  4. Thorough Testing

    • Unit tests for Ed25519 signing, validation, and header injection
    • CLI integration test for auth binding add --signer-json with Ed25519 config
    • Skill validation script checks all required documentation patterns
    • Multiple CI checks passing (Test (beta), Test (nightly), Rust Quality, Security Audit, Validate (binance-spot-openapi-skill))
  5. Documentation Quality

    • Clear distinction between Ed25519 (recommended) and HMAC (legacy) flows
    • Explicit warnings about the -1022 signature mismatch pitfall
    • Testnet-first guardrails for write operations
    • Comprehensive usage examples in references/usage-patterns.md

Issues

Must Fix (None)

No correctness bugs, security issues, or API compatibility concerns identified.

Should Fix

  1. Algorithm Validation Inconsistency (src/auth/mod.rs:2186-2188)

    • hmac_query_v1 now validates that algorithm == HmacSha256
    • This is a good safety check, but consider whether this should be a documented breaking change for existing bindings that may have omitted the field
    • Suggestion: Add a migration note or make the validation more permissive if the field is missing
    // Current code requires explicit algorithm=hmac_sha256
    if hmac.algorithm != SignerAlgorithm::HmacSha256 {
        anyhow::bail!("hmac_query_v1 signer requires algorithm=hmac_sha256");
    }
  2. Test Key Exposure (src/auth/mod.rs:2424)

    • The test Ed25519 private key PEM is hardcoded in the test module
    • While this is acceptable for a testnet-only key, consider adding a comment clarifying it's a test key
    • Suggestion: Add // Test key for Binance Spot testnet only - never used in production comment

Optional Improvements

  1. SKILL.md OpenAI Agent Reference (skills/binance-spot-openapi-skill/agents/openai.yaml:3-4)

    • The short_description mentions "HMAC query signing" but Ed25519 is now the recommended approach
    • Suggestion: Update to "HMAC or Ed25519 query signing"
    short_description: "Operate Binance Spot public market, signed account, and order endpoints via UXC with curated OpenAPI mapping and HMAC/Ed25519 query signing"
  2. CLI Help Example Length (src/main.rs:2752)

    • The Ed25519 --signer-json example is quite long and may wrap on narrower terminals
    • Suggestion: Consider breaking it across multiple lines or using a more concise example
  3. Error Message Clarity (src/auth/mod.rs:2225-2227)

    • When key_field, key_placement, and key_name are not all set together, the error message could be more actionable
    • Suggestion: Include the actual values found to help users debug their config
    anyhow::bail!(
        "ed25519_query_v1 signer requires key_field, key_placement, and key_name to be set together (found: key_field={:?}, key_placement={:?}, key_name={:?})",
        ed25519.key_field.is_some(),
        ed25519.key_placement,
        ed25519.key_name
    );

Testing Coverage

Passing Tests

  • ✅ Unit tests: Ed25519 signing, validation, header injection
  • ✅ CLI integration: auth binding add --signer-json with Ed25519
  • ✅ Skill validation: Required files, schema structure, documentation patterns
  • ✅ CI checks: Test (beta), Test (nightly), Rust Quality, Security Audit, Validate (binance-spot-openapi-skill)

Manual Testing Noted

The PR description mentions live Binance Spot testnet verification for:

  • get:/api/v3/account
  • post:/api/v3/order/test

This is excellent practice for a financial API integration.

Security Considerations

  1. Private Key Handling

    • Ed25519 private keys are loaded from environment variables or file reads
    • Keys are trimmed before parsing to handle common PEM formatting issues
    • No key material is logged or exposed in error messages
  2. Signature Validation

    • The implementation correctly validates that Ed25519 signatures must use base64 encoding
    • This prevents accidental hex encoding which would cause Binance API failures
  3. Testnet Guardrails

    • Documentation strongly prefers testnet for all examples
    • Explicit warnings about mainnet write operations requiring user confirmation

Compatibility

Backward Compatibility

  • ✅ HMAC hmac_query_v1 signer remains unchanged
  • ✅ Existing auth bindings continue to work
  • ⚠️ New algorithm field validation may reject existing HMAC configs that omitted the field (see Should Fix Setup CI/CD pipeline (GitHub Actions) #1)

Breaking Changes

None intentionally introduced, but the stricter HMAC algorithm validation could affect existing configurations.

Recommendations

Before Merge

  1. Address the HMAC algorithm validation concern (Should Fix Setup CI/CD pipeline (GitHub Actions) #1)
  2. Add clarifying comment for test Ed25519 key (Should Fix Implement OpenAPI schema parser #2)

After Merge

  1. Update OpenAI agent description to mention Ed25519 (Optional Setup CI/CD pipeline (GitHub Actions) #1)
  2. Consider improving Ed25519 config validation error messages (Optional Implement protocol detection #3)

Conclusion

This is a high-quality PR that successfully adds Ed25519 support for Binance Spot API integration while maintaining backward compatibility with HMAC. The OpenAPI auth resolution fix is a valuable improvement that prevents a subtle bug. The comprehensive testing and documentation demonstrate strong engineering practices.

Recommendation: APPROVE after addressing the two should-fix items above.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds Binance Spot OpenAPI skill support and extends UXC’s auth/signing pipeline to support Binance’s Ed25519 query-signing flow, while tightening auth resolution for OpenAPI operations and preventing business auth from contaminating schema-fetch requests.

Changes:

  • Add ed25519_query_v1 signer (PKCS#8 PEM Ed25519, base64 signatures) alongside existing HMAC query signer, plus CLI/unit tests.
  • Adjust daemon/OpenAPI auth application so OpenAPI execution resolves auth against the final operation URL and schema URL fetching avoids business auth injection.
  • Add new binance-spot-openapi-skill package (schema, docs, usage patterns, validation script).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/auth_binding_cli_test.rs Adds CLI test coverage for --signer-json with ed25519_query_v1.
src/main.rs Updates help text/examples to include ed25519_query_v1.
src/daemon.rs Separates detection-time vs execution-time auth for OpenAPI; resolves auth using final operation URL.
src/auth/mod.rs Implements Ed25519 query signer config/validation and signing; refactors signer header handling.
src/adapters/openapi.rs Prevents auth being applied to schema URL override requests; adds regression test.
skills/binance-spot-openapi-skill/scripts/validate.sh Adds skill package validation checks (files, docs patterns, schema assertions).
skills/binance-spot-openapi-skill/references/usage-patterns.md Documents link/auth/signing flows for mainnet/testnet with Ed25519 + HMAC.
skills/binance-spot-openapi-skill/references/binance-spot.openapi.json Adds curated OpenAPI 3.1 schema for key Spot endpoints.
skills/binance-spot-openapi-skill/agents/openai.yaml Adds agent metadata for the new skill.
skills/binance-spot-openapi-skill/SKILL.md Adds primary skill documentation and operational guidance/guardrails.
Cargo.toml Adds ed25519-dalek dependency (pkcs8/pem features).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +131 to +155
uxc auth credential set binance-spot-mainnet \
--auth-type api_key \
--field api_key=env:BINANCE_MAINNET_API_KEY \
--field secret_key=env:BINANCE_MAINNET_SECRET_KEY

uxc auth credential set binance-spot-testnet \
--auth-type api_key \
--field api_key=env:BINANCE_TESTNET_API_KEY \
--field secret_key=env:BINANCE_TESTNET_SECRET_KEY

uxc auth binding add \
--id binance-spot-mainnet \
--host api.binance.com \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-mainnet \
--signer-json '{"kind":"hmac_query_v1","algorithm":"hmac_sha256","signing_field":"secret_key","key_field":"api_key","key_placement":"header","key_name":"X-MBX-APIKEY","signature_param":"signature","signature_encoding":"hex","timestamp_param":"timestamp","timestamp_unit":"milliseconds","canonicalization":{"mode":"preserve_order"}}' \
--priority 100

uxc auth binding add \
--id binance-spot-testnet \
--host testnet.binance.vision \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-testnet \
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The HMAC fallback commands reuse the same credential IDs (binance-spot-mainnet / binance-spot-testnet) as the recommended Ed25519 setup above, so following this section would overwrite the Ed25519 credentials. Use distinct IDs (e.g. *-hmac) or explicitly call out that this replaces the earlier credentials.

Suggested change
uxc auth credential set binance-spot-mainnet \
--auth-type api_key \
--field api_key=env:BINANCE_MAINNET_API_KEY \
--field secret_key=env:BINANCE_MAINNET_SECRET_KEY
uxc auth credential set binance-spot-testnet \
--auth-type api_key \
--field api_key=env:BINANCE_TESTNET_API_KEY \
--field secret_key=env:BINANCE_TESTNET_SECRET_KEY
uxc auth binding add \
--id binance-spot-mainnet \
--host api.binance.com \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-mainnet \
--signer-json '{"kind":"hmac_query_v1","algorithm":"hmac_sha256","signing_field":"secret_key","key_field":"api_key","key_placement":"header","key_name":"X-MBX-APIKEY","signature_param":"signature","signature_encoding":"hex","timestamp_param":"timestamp","timestamp_unit":"milliseconds","canonicalization":{"mode":"preserve_order"}}' \
--priority 100
uxc auth binding add \
--id binance-spot-testnet \
--host testnet.binance.vision \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-testnet \
uxc auth credential set binance-spot-mainnet-hmac \
--auth-type api_key \
--field api_key=env:BINANCE_MAINNET_API_KEY \
--field secret_key=env:BINANCE_MAINNET_SECRET_KEY
uxc auth credential set binance-spot-testnet-hmac \
--auth-type api_key \
--field api_key=env:BINANCE_TESTNET_API_KEY \
--field secret_key=env:BINANCE_TESTNET_SECRET_KEY
uxc auth binding add \
--id binance-spot-mainnet-hmac \
--host api.binance.com \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-mainnet-hmac \
--signer-json '{"kind":"hmac_query_v1","algorithm":"hmac_sha256","signing_field":"secret_key","key_field":"api_key","key_placement":"header","key_name":"X-MBX-APIKEY","signature_param":"signature","signature_encoding":"hex","timestamp_param":"timestamp","timestamp_unit":"milliseconds","canonicalization":{"mode":"preserve_order"}}' \
--priority 100
uxc auth binding add \
--id binance-spot-testnet-hmac \
--host testnet.binance.vision \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-testnet-hmac \

Copilot uses AI. Check for mistakes.
Comment on lines +131 to +155
uxc auth credential set binance-spot-mainnet \
--auth-type api_key \
--field api_key=env:BINANCE_MAINNET_API_KEY \
--field secret_key=env:BINANCE_MAINNET_SECRET_KEY

uxc auth credential set binance-spot-testnet \
--auth-type api_key \
--field api_key=env:BINANCE_TESTNET_API_KEY \
--field secret_key=env:BINANCE_TESTNET_SECRET_KEY

uxc auth binding add \
--id binance-spot-mainnet \
--host api.binance.com \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-mainnet \
--signer-json '{"kind":"hmac_query_v1","algorithm":"hmac_sha256","signing_field":"secret_key","key_field":"api_key","key_placement":"header","key_name":"X-MBX-APIKEY","signature_param":"signature","signature_encoding":"hex","timestamp_param":"timestamp","timestamp_unit":"milliseconds","canonicalization":{"mode":"preserve_order"}}' \
--priority 100

uxc auth binding add \
--id binance-spot-testnet \
--host testnet.binance.vision \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-testnet \
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The HMAC fallback binding examples reuse the same binding IDs and credentials as the Ed25519 section (binance-spot-mainnet/binance-spot-testnet). That can unintentionally replace the Ed25519 binding or mix signers depending on how bindings are stored/updated. Use distinct binding/credential IDs for HMAC (e.g. *-hmac) or document the required removal/replace step.

Suggested change
uxc auth credential set binance-spot-mainnet \
--auth-type api_key \
--field api_key=env:BINANCE_MAINNET_API_KEY \
--field secret_key=env:BINANCE_MAINNET_SECRET_KEY
uxc auth credential set binance-spot-testnet \
--auth-type api_key \
--field api_key=env:BINANCE_TESTNET_API_KEY \
--field secret_key=env:BINANCE_TESTNET_SECRET_KEY
uxc auth binding add \
--id binance-spot-mainnet \
--host api.binance.com \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-mainnet \
--signer-json '{"kind":"hmac_query_v1","algorithm":"hmac_sha256","signing_field":"secret_key","key_field":"api_key","key_placement":"header","key_name":"X-MBX-APIKEY","signature_param":"signature","signature_encoding":"hex","timestamp_param":"timestamp","timestamp_unit":"milliseconds","canonicalization":{"mode":"preserve_order"}}' \
--priority 100
uxc auth binding add \
--id binance-spot-testnet \
--host testnet.binance.vision \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-testnet \
uxc auth credential set binance-spot-mainnet-hmac \
--auth-type api_key \
--field api_key=env:BINANCE_MAINNET_API_KEY \
--field secret_key=env:BINANCE_MAINNET_SECRET_KEY
uxc auth credential set binance-spot-testnet-hmac \
--auth-type api_key \
--field api_key=env:BINANCE_TESTNET_API_KEY \
--field secret_key=env:BINANCE_TESTNET_SECRET_KEY
uxc auth binding add \
--id binance-spot-mainnet-hmac \
--host api.binance.com \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-mainnet-hmac \
--signer-json '{"kind":"hmac_query_v1","algorithm":"hmac_sha256","signing_field":"secret_key","key_field":"api_key","key_placement":"header","key_name":"X-MBX-APIKEY","signature_param":"signature","signature_encoding":"hex","timestamp_param":"timestamp","timestamp_unit":"timestamp","timestamp_unit":"milliseconds","canonicalization":{"mode":"preserve_order"}}' \
--priority 100
uxc auth binding add \
--id binance-spot-testnet-hmac \
--host testnet.binance.vision \
--path-prefix /api/v3 \
--scheme https \
--credential binance-spot-testnet-hmac \

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +4
short_description: "Operate Binance Spot public market, signed account, and order endpoints via UXC with curated OpenAPI mapping and HMAC query signing"
default_prompt: "Use $binance-spot-openapi-skill to discover and execute Binance Spot REST operations through UXC with separate mainnet/testnet link flows, HMAC signer bindings, and testnet-first write guardrails."
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

This agent metadata describes the skill as using only “HMAC query signing” / “HMAC signer bindings”, but the PR adds and recommends Ed25519 as well. Update the short_description/default_prompt to reflect both signers (or emphasize Ed25519 as recommended) to avoid misleading users.

Suggested change
short_description: "Operate Binance Spot public market, signed account, and order endpoints via UXC with curated OpenAPI mapping and HMAC query signing"
default_prompt: "Use $binance-spot-openapi-skill to discover and execute Binance Spot REST operations through UXC with separate mainnet/testnet link flows, HMAC signer bindings, and testnet-first write guardrails."
short_description: "Operate Binance Spot public market, signed account, and order endpoints via UXC with curated OpenAPI mapping and Ed25519 (recommended) and HMAC query signing"
default_prompt: "Use $binance-spot-openapi-skill to discover and execute Binance Spot REST operations through UXC with separate mainnet/testnet link flows, Ed25519 (recommended) and HMAC signer bindings, and testnet-first write guardrails."

Copilot uses AI. Check for mistakes.
src/main.rs Outdated
commands: vec![],
notes: vec![
"--signer-json attaches a typed signer config to this binding, for example kind=hmac_query_v1.".to_string(),
"--signer-json attaches a typed signer config to this binding, for example kind=hmac_query_v1 or kind=ed25519_query_v1.".to_string(),
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The note says “for example kind=hmac_query_v1 ...”, but --signer-json expects a JSON object (as shown in the examples). Consider rephrasing to something like “e.g. {"kind":"hmac_query_v1", ...}” to avoid implying a non-JSON format is accepted.

Suggested change
"--signer-json attaches a typed signer config to this binding, for example kind=hmac_query_v1 or kind=ed25519_query_v1.".to_string(),
"--signer-json attaches a typed signer config to this binding, e.g. {\"kind\":\"hmac_query_v1\", ...} or {\"kind\":\"ed25519_query_v1\", ...}.".to_string(),

Copilot uses AI. Check for mistakes.
@jolestar jolestar force-pushed the feature/binance-spot-ed25519 branch from 040f6a5 to d1521e4 Compare March 9, 2026 07:40
@jolestar jolestar merged commit dcc5cce into main Mar 9, 2026
20 checks passed
@jolestar jolestar deleted the feature/binance-spot-ed25519 branch March 9, 2026 08:06
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.

2 participants