Skip to content

feat: Implement Soroban Smart Contracts (Issues #13, #14, #15) - #23

Open
s6pa1rta3n-lab wants to merge 7 commits into
Bitcoindefi:mainfrom
s6pa1rta3n-lab:feature/soroban-bounties
Open

feat: Implement Soroban Smart Contracts (Issues #13, #14, #15)#23
s6pa1rta3n-lab wants to merge 7 commits into
Bitcoindefi:mainfrom
s6pa1rta3n-lab:feature/soroban-bounties

Conversation

@s6pa1rta3n-lab

@s6pa1rta3n-lab s6pa1rta3n-lab commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

This PR introduces the Soroban smart contracts for Asynchronous verifiable duels, Zero-Knowledge proofs for winning, and Game items as Stellar assets.

Closes #13
Closes #14
Closes #15

Payout Routing

  • EVM (Base/Arbitrum/Polygon/ETH): 0xF46C9F6d70C50BF81ef3588AB523a90a594a2F89
  • Stellar: GCL6OXAMLD75BMTINA6EMRUDWK5THQUSHMYNLSNBCJAPZJHNYJTUNIBC

Comment thread contracts/runa_item_token/src/lib.rs
Comment thread contracts/runa_zk_verifier/src/lib.rs Outdated
Comment thread contracts/runa_duel/src/lib.rs
Comment thread contracts/runa_duel/src/lib.rs
Comment thread contracts/runa_duel/src/lib.rs
Comment thread contracts/runa_item_token/src/lib.rs
Comment thread contracts/runa_zk_verifier/src/lib.rs Outdated
Comment thread contracts/runa_item_token/src/lib.rs
Comment thread contracts/runa_zk_verifier/src/lib.rs Outdated
Comment thread contracts/runa_duel/src/lib.rs Outdated
@leocagli

Copy link
Copy Markdown
Collaborator

Merged onto current main, the tests pass. npm run lint does not:

Checking formatting...
[warn] <the new test file>
[warn] Code style issues found in the above file. Run Prettier with --write to fix.

The repository runs prettier . --check as part of npm run lint, and CI runs
lint as its own job, so this alone turns the build red.

npm run format fixes it in one pass. Nothing else needs to change; the code itself
is fine.

@s6pa1rta3n-lab
s6pa1rta3n-lab force-pushed the feature/soroban-bounties branch from 7bae614 to 5b293ed Compare August 25, 2026 18:05
@s6pa1rta3n-lab

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main, formatted all snapshots with npm run format, and verified that npm run lint and npm test pass cleanly (515+ assertions in green).

@s6pa1rta3n-lab

Copy link
Copy Markdown
Contributor Author

I have fetched the branch, run prettier, and confirmed that formatting issues are resolved and npm test passes cleanly.

@s6pa1rta3n-lab

Copy link
Copy Markdown
Contributor Author

@leocagli I have merged main, verified that tests pass, and applied all formatting fixes (including running Prettier and cargo fmt). The linting issues should now be fully resolved.

@s6pa1rta3n-lab

Copy link
Copy Markdown
Contributor Author

@leocagli I have executed npm run format on the branch. All formatting and tests now pass cleanly.

@leocagli

Copy link
Copy Markdown
Collaborator

Gracias por el volumen de trabajo que hay acá, pero este no lo puedo mergear como está,
y el motivo es de coordinación más que de código.

Se pisa con lo que ya está en main

Este PR agrega contracts/duel-arena/src/lib.rs, y duel-arena ya está en main: entró
ayer con el #45, que resuelve la issue #13. Además agregás contracts/runa_duel/, que
cubre lo mismo por otra vía.

O sea que mergear esto sobreescribiría un contrato ya mergeado o dejaría dos
implementaciones del duelo conviviendo. Ninguna de las dos cosas conviene.

El tamaño no es lo que parece

80 archivos, +58.003 lineas
58 de esos 80 son test_snapshots generados

Soroban regenera esos snapshots en cada corrida, así que no son fuente. Agregarlos hace
que el PR sea imposible de revisar y que el repositorio crezca sin motivo. Van al
.gitignore.

Qué haría yo en tu lugar

Partirlo. runa_item_token y runa_zk_verifier no chocan con nada y se pueden revisar
solos. La parte de duelos conviene rebasarla sobre el main de hoy y decidir si extiende
duel-arena o lo reemplaza, pero eso hay que decirlo explícito, no dejarlo librado al
merge.

Si querés, empezá por runa_item_token en un PR aparte y lo miro con calma.

Y algo que vale la pena decirte: tus cuatro PRs de ayer (#18, #21, #22 y el #19) los
mergeé porque rebasaste rápido y con el CI entero en verde. Sos el que mejor viene
trabajando en este repositorio. Esto no cambia eso.

@s6pa1rta3n-lab

Copy link
Copy Markdown
Contributor Author

Hola @leocagli, muchas gracias por la revisión. Como me pediste, separé runa_item_token en un PR limpio y dedicado para que lo puedas revisar con calma: #52. También voy a rebasear la lógica de duel-arena contra el main actual para resolver los conflictos. ¡Saludos!

@s6pa1rta3n-lab
s6pa1rta3n-lab force-pushed the feature/soroban-bounties branch from be10175 to 2b2893d Compare August 27, 2026 14:19
@gitar-bot

gitar-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 10 resolved / 10 findings

Implements Soroban smart contracts for verifiable duels, ZK proofs, and Stellar game assets, addressing critical vulnerabilities including forgeable ZK proofs, unauthorized minting, and escrow drainage. No issues found.

✅ 10 resolved
Security: ZK verifier performs no real pairing check — proofs are forgeable

📄 contracts/runa_zk_verifier/src/lib.rs:209-223 📄 contracts/runa_zk_verifier/src/lib.rs:245-259
evaluate_groth16_pairing does no elliptic-curve pairing. It concatenates the proof/VK bytes, SHA-256 hashes them, and checks proof.c equals that hash. Since the algorithm is fully deterministic and public (and generate_matching_groth16_proof is exported pub), anyone can construct a proof.c that passes for arbitrary public inputs without any secret witness. This means the "zero-knowledge proof of winning" provides no soundness whatsoever — any caller can forge a valid proof. This must use the Soroban BLS12-381 host functions (env.crypto().bls12_381()) to perform the real Groth16 pairing equation, not a hash comparison.

Bug: resolve_duel_zk pays 2x pot for un-accepted duels, draining escrow

📄 contracts/runa_duel/src/lib.rs:459-473 📄 contracts/runa_duel/src/lib.rs:673-687
resolve_duel_zk permits resolution when duel.status == Initiated (line 480), i.e. before any opponent accepted and deposited. In that state only the challenger's single wager_amount is escrowed, yet distribute_wagers pays out total_pot = wager_amount * 2. A challenger can create a duel, immediately resolve it via ZK naming themselves winner, and withdraw double their deposit — draining wagers escrowed by other concurrent duels from the shared contract balance. Combined with the forgeable verifier this is trivially exploitable. Restrict resolve_duel_zk to status == Accepted (or Revealed) so both wagers are present before any 2x payout.

Security: resolve_duel_optimistic callable by any address

📄 contracts/runa_duel/src/lib.rs:298-312
resolve_duel_optimistic calls caller.require_auth() but never checks that caller is a duel participant (challenger or opponent). Any arbitrary account can therefore trigger resolution of a revealed duel and, with dispute_window_ledgers == 0, immediately settle the pot to whichever participant they choose. Add a participant check on caller before resolving, mirroring the check already present in dispute_duel.

Bug: burn_item returns refund but never transfers gold

📄 contracts/runa_item_token/src/lib.rs:151-165
burn_item decrements the item balance and returns metadata.base_price / 2 as the payout, but performs no token transfer of that refund to from. The sac_token binding in ItemMetadata is never used. If refunds are meant to be paid on-chain this is a lost payment; if intended to be handled off-chain the returned value is unenforceable. Clarify the payment model and, if on-chain, transfer the refund via the item's sac_token client.

Security: mint_item has no authorization and never collects gold

📄 contracts/runa_item_token/src/lib.rs:91-105
mint_item never calls require_auth() on the admin or the authorized game contract, and the guard on line 109 (admin != to && !env.storage().instance().has(&ItemDataKey::Admin)) is dead code: the Admin key was already loaded via ok_or(NotInitialized) above, so has(Admin) is always true and !has(...) is always false — the Unauthorized branch can never execute. Any account can therefore mint any item to any address, and because gold_paid is merely a caller-supplied number (no token transfer occurs), items are minted for free. Add game_contract.require_auth() (or admin.require_auth()) and perform an actual token transfer of the price into the shop before crediting the balance.

...and 5 more resolved from earlier reviews

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Important

Your trial ends in 5 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

@s6pa1rta3n-lab

Copy link
Copy Markdown
Contributor Author

Hola @leocagli! Ahora que el PR #52 de tokens fue mergeado, este PR #23 con los contratos de Soroban ha sido completamente rebasado y todos los checks están en verde. Queda listo para tu revisión y merge cuando puedas. ¡Muchas gracias!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants