Origin/arkhe-origin-chain
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Activity

fix(contracts): FIX-1 contract-safety + migración ink! v6 (escrow/staking/nft)

Browse Source 
REMEDIATION_ROADMAP_V1 FIX-1 (V1-C1 reentrancy + V1-C4 zero-burn). Los 3
templates ahora COMPILAN contra ink! 6.0.0-beta.1 + tests verde (12/12).

Hallazgo (honesto): los templates se shippearon "scaffold compile-only" pero
nunca compilaron contra su versión pinneada de ink!. ink! v6 / pallet-revive es
H160-Address + U256 nativo (no AccountId + Balance, modelo viejo). Esta
migración los vuelve reales (compilando + testeados).

Migración ink! v6:
- AccountId -> Address (H160) en parties / storage / events / reads / params.
- transfer() + transferred_value() -> U256 (+ helper u256_to_balance saturante).
- derive StorageLayout en tipos de storage (EscrowState/Milestone/Stake/...).
- doctest fences sin lenguaje -> text (diagramas ASCII del lifecycle).
- workspace: contracts/* excluidos (toolchain PolkaVM/pallet-revive distinta).

FIX-1 hardening:
- CEI (Checks-Effects-Interactions): muta TODO el storage ANTES de cualquier
  transfer() -> una reentrada ve el estado final y falla sus checks. Sitios:
  escrow confirm_milestone/resolve_dispute, staking withdraw/slash,
  nft mint/transfer. Cierra V1-C1. (nft mint encontrado más allá de la lista
  del roadmap; mismo patrón explotable -> incluido).
- Burn real (V1-C4): elimina transfer([0;32]) (que no reduce emisión y ensucia
  la cuenta cero); el burn se RETIENE en el contrato (fuera de circulación),
  a barrer por el primitivo de burn del runtime -> una sola semántica, no dos.
  (staking slash, nft mint).
- Tests de guarda de estado (la C de CEI) añadidos en los 3 contratos.

Pendiente E2E (node-gated, requiere VPS + nodo pallet-revive):
- Correctitud económica native<->eth (NATIVE_TO_ETH_RATIO): asumido 1:1; debe
  validarse en nodo real antes de testnet/mainnet.
- Tests reales de reentrancy/double-spend: el env off-chain de #[ink::test] no
  modela el revert-on-Err ni la semántica de balances -> serían engañosos.
- V1-H7 (suma milestones == total_amount): no incluido en este commit.
- credential-mint + mdk-reconciliation necesitan la misma migración ink! v6.

Cross-pillar: DC ORIGIN_ATTESTATION_SIGNING_PAYLOAD_V1 abierto con Ergon
(payload postcard canónico + domain-separation FIX-3) — gate del verificador
de attestation on-chain. OAGIP-D10 (Charon identity attestation) definido a
Palace para el protocolo.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
davidherranandClaude Opus 4.8 committed 2026-06-09 17:45:02 -05:00
1 parent a15a65966f
commit 6663dbdc0f
5 files changed
+325 -149
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

contracts/origin-escrow/lib.rs
+139 -59
View File
@@ -29,7 +29,7 @@
//!
//! ## Lifecycle canonical (5 states)
//!
//! ```
//! ```text
//! Created ──► Active ──► Releasing ──► Completed
//! │ │ │
//! │ ▼ ▼
@@ -58,8 +58,8 @@
#[ink::contract]
mod origin_escrow {
use ink::prelude::vec::Vec;
use ink::storage::Mapping;
use ink::U256;
pub type MilestoneId = u32;
pub type BasisPoints = u16;
@@ -70,7 +70,7 @@ mod origin_escrow {
// ─────────────────────────────────────────────────────────────────
#[derive(Debug, Clone, Copy, PartialEq, Eq, scale::Encode, scale::Decode)]
#[cfg_attr(feature = "std", derive(scale_info::TypeInfo))]
#[cfg_attr(feature = "std", derive(scale_info::TypeInfo, ink::storage::traits::StorageLayout))]
pub enum EscrowState {
Created,
Active,
@@ -81,7 +81,7 @@ mod origin_escrow {
/// Milestone canonical structure — define partial releases del total_amount.
#[derive(Debug, Clone, scale::Encode, scale::Decode)]
#[cfg_attr(feature = "std", derive(scale_info::TypeInfo))]
#[cfg_attr(feature = "std", derive(scale_info::TypeInfo, ink::storage::traits::StorageLayout))]
pub struct Milestone {
pub amount: Balance,
pub due_block: BlockNumber,
@@ -107,10 +107,10 @@ mod origin_escrow {
#[ink(storage)]
pub struct OriginEscrow {
// ── Parties + config ─────────────────────────────────────
depositor: AccountId,
beneficiary: AccountId,
arbiter: AccountId,
treasury: AccountId,
depositor: Address,
beneficiary: Address,
arbiter: Address,
treasury: Address,
terms_hash: TermsHash,
total_amount: Balance,
escrow_fee_bps: BasisPoints,
@@ -123,7 +123,7 @@ mod origin_escrow {
deposited_amount: Balance,
released_amount: Balance,
dispute_raised_at: Option<BlockNumber>,
dispute_raised_by: Option<AccountId>,
dispute_raised_by: Option<Address>,
last_release_block: Option<BlockNumber>,
// ── Milestones canonical (partial releases) ─────────────
@@ -138,7 +138,7 @@ mod origin_escrow {
#[ink(event)]
pub struct Deposited {
#[ink(topic)]
depositor: AccountId,
depositor: Address,
amount: Balance,
}
@@ -152,15 +152,15 @@ mod origin_escrow {
#[ink(event)]
pub struct DisputeRaised {
#[ink(topic)]
raised_by: AccountId,
raised_by: Address,
#[ink(topic)]
arbiter: AccountId,
arbiter: Address,
}
#[ink(event)]
pub struct DisputeResolved {
#[ink(topic)]
arbiter: AccountId,
arbiter: Address,
resolution: Resolution,
depositor_refund: Balance,
beneficiary_release: Balance,
@@ -196,16 +196,32 @@ mod origin_escrow {
pub type Result<T> = core::result::Result<T, Error>;
/// Convierte un valor `U256` del entorno pallet-revive a `Balance` (u128)
/// de forma saturante.
///
/// ⚠️ NOTA de correctitud económica: pallet-revive maneja dos
/// representaciones (nativa vs eth, `NATIVE_TO_ETH_RATIO`). Aquí se asume
/// representación nativa 1:1 (lo que el doc-example de `transfer` sugiere).
/// La equivalencia exacta DEBE validarse E2E en un nodo pallet-revive real
/// antes de cualquier anclaje en testnet/mainnet — pendiente VPS.
fn u256_to_balance(v: U256) -> Balance {
if v > U256::from(Balance::MAX) {
Balance::MAX
} else {
v.as_u128()
}
}
impl OriginEscrow {
/// Constructor canonical con full parameters.
///
/// Invariant compile-detect: escrow_fee_bps ≤ 500 (5% max ceiling BM V2 §5.3).
#[ink(constructor)]
pub fn new(
depositor: AccountId,
beneficiary: AccountId,
arbiter: AccountId,
treasury: AccountId,
depositor: Address,
beneficiary: Address,
arbiter: Address,
treasury: Address,
terms_hash: TermsHash,
total_amount: Balance,
escrow_fee_bps: BasisPoints,
@@ -251,13 +267,13 @@ mod origin_escrow {
pub fn state(&self) -> EscrowState { self.state }
#[ink(message)]
pub fn depositor(&self) -> AccountId { self.depositor }
pub fn depositor(&self) -> Address { self.depositor }
#[ink(message)]
pub fn beneficiary(&self) -> AccountId { self.beneficiary }
pub fn beneficiary(&self) -> Address { self.beneficiary }
#[ink(message)]
pub fn arbiter(&self) -> AccountId { self.arbiter }
pub fn arbiter(&self) -> Address { self.arbiter }
#[ink(message)]
pub fn terms_hash(&self) -> TermsHash { self.terms_hash }
@@ -291,7 +307,7 @@ mod origin_escrow {
if self.env().caller() != self.depositor {
return Err(Error::UnauthorizedCaller);
}
let value = self.env().transferred_value();
let value = u256_to_balance(self.env().transferred_value());
if value < self.total_amount {
return Err(Error::InsufficientDeposit);
}
@@ -354,30 +370,41 @@ mod origin_escrow {
return Err(Error::EarlyReleaseProhibited);
}
// Release canonical: amount - escrow_fee → beneficiary
// Release: amount - escrow_fee → beneficiary.
let fee = milestone.amount
.saturating_mul(self.escrow_fee_bps as Balance)
.saturating_div(10_000);
let net = milestone.amount.saturating_sub(fee);
self.env().transfer(self.beneficiary, net).map_err(|_| Error::TransferFailed)?;
self.env().transfer(self.treasury, fee).map_err(|_| Error::TransferFailed)?;
// FIX-1 (V1-C1) Checks-Effects-Interactions: TODO el storage muta
// ANTES de cualquier transfer(), para que una reentrada vea el estado
// final (milestone ya completado) y falle sus checks.
//
// ── EFFECTS ──
milestone.completed = true;
milestone.completed_at = Some(now);
self.milestones.insert(milestone_id, &milestone);
self.released_amount = self.released_amount.saturating_add(milestone.amount);
self.last_release_block = Some(now);
let completed = self.released_amount >= self.total_amount;
if completed {
self.state = EscrowState::Completed;
}
// ── INTERACTIONS (transfers al final) ──
self.env()
.transfer(self.beneficiary, U256::from(net))
.map_err(|_| Error::TransferFailed)?;
self.env()
.transfer(self.treasury, U256::from(fee))
.map_err(|_| Error::TransferFailed)?;
// ── EVENTS ──
self.env().emit_event(MilestoneCompleted {
milestone_id,
amount_released: net,
});
// Auto-transition Completed if all milestones canonical released
if self.released_amount >= self.total_amount {
self.state = EscrowState::Completed;
if completed {
self.env().emit_event(EscrowCompleted {
total_released: self.released_amount,
total_refunded: 0,
@@ -447,15 +474,28 @@ mod origin_escrow {
}
};
// FIX-1 (V1-C1) CEI: estado terminal ANTES de los transfers, para que
// una reentrada vea state=Completed y falle el check de InvalidState.
//
// ── EFFECTS ──
self.state = EscrowState::Completed;
// ── INTERACTIONS ──
if depositor_refund > 0 {
self.env().transfer(self.depositor, depositor_refund).map_err(|_| Error::TransferFailed)?;
self.env()
.transfer(self.depositor, U256::from(depositor_refund))
.map_err(|_| Error::TransferFailed)?;
}
if beneficiary_release > 0 {
self.env().transfer(self.beneficiary, beneficiary_release).map_err(|_| Error::TransferFailed)?;
self.env()
.transfer(self.beneficiary, U256::from(beneficiary_release))
.map_err(|_| Error::TransferFailed)?;
}
self.env().transfer(self.treasury, fee).map_err(|_| Error::TransferFailed)?;
self.env()
.transfer(self.treasury, U256::from(fee))
.map_err(|_| Error::TransferFailed)?;
self.state = EscrowState::Completed;
// ── EVENTS ──
self.env().emit_event(DisputeResolved {
arbiter: self.arbiter,
resolution,
@@ -476,53 +516,93 @@ mod origin_escrow {
mod tests {
use super::*;
#[ink::test]
fn constructor_canonical_invariants() {
let result = OriginEscrow::new(
AccountId::from([1u8; 32]),
AccountId::from([2u8; 32]),
AccountId::from([3u8; 32]),
AccountId::from([4u8; 32]),
/// Contrato válido en estado `Created` (default test caller = owner).
fn mk() -> OriginEscrow {
OriginEscrow::new(
Address::from([1u8; 20]),
Address::from([2u8; 20]),
Address::from([3u8; 20]),
Address::from([4u8; 20]),
[1u8; 32],
1_000_000_000_000_000_000,
150, // 1.5% canonical BM V2
150, // 1.5% BM V2
100,
1000,
false,
);
assert!(result.is_ok());
let escrow = result.unwrap();
assert_eq!(escrow.state(), EscrowState::Created);
assert_eq!(escrow.deposited_amount(), 0);
)
.expect("valid config")
}
#[ink::test]
fn excessive_fee_rejected_canonical() {
fn constructor_invariants() {
let escrow = mk();
assert_eq!(escrow.state(), EscrowState::Created);
assert_eq!(escrow.deposited_amount(), 0);
assert_eq!(escrow.released_amount(), 0);
}
#[ink::test]
fn excessive_fee_rejected() {
let result = OriginEscrow::new(
AccountId::from([1u8; 32]),
AccountId::from([2u8; 32]),
AccountId::from([3u8; 32]),
AccountId::from([4u8; 32]),
Address::from([1u8; 20]),
Address::from([2u8; 20]),
Address::from([3u8; 20]),
Address::from([4u8; 20]),
[1u8; 32],
1_000_000_000_000_000_000,
600, // 6% — exceeds 5% max BM V2 ceiling
100, 1000, false,
600, // 6% — exceeds 5% max BM V2 ceiling
100,
1000,
false,
);
assert_eq!(result.err(), Some(Error::InvalidConfiguration));
}
#[ink::test]
fn same_party_depositor_beneficiary_rejected() {
let same = AccountId::from([1u8; 32]);
let same = Address::from([1u8; 20]);
let result = OriginEscrow::new(
same, same,
AccountId::from([3u8; 32]),
AccountId::from([4u8; 32]),
same,
same,
Address::from([3u8; 20]),
Address::from([4u8; 20]),
[1u8; 32],
1_000_000_000_000_000_000,
150, 100, 1000, false,
150,
100,
1000,
false,
);
assert_eq!(result.err(), Some(Error::InvalidConfiguration));
}
/// FIX-1 (V1-C1) — guarda de estado: ninguna acción que mueva fondos
/// (`confirm_milestone` / `raise_dispute` / `resolve_dispute`) es
/// alcanzable desde un estado equivocado. Es la "C" (Checks) del patrón
/// CEI — el primer guard corta ANTES de llegar a effects/interactions.
///
/// ⚠️ Los tests E2E de reentrancy / double-spend reales (que ejercitan
/// los `transfer()` y el revert-on-Err del pallet) están node-gated:
/// requieren cargo-contract v6 + un nodo pallet-revive (VPS). El env
/// off-chain de `#[ink::test]` NO modela el revert-on-Err ni la
/// semántica native↔eth de balances, por lo que un test de transfer
/// aquí sería engañoso. El fix CEI es correctness-by-construction
/// (red-team ratificado); su validación dinámica espera el nodo real.
#[ink::test]
fn fund_moving_actions_blocked_from_created_state() {
let mut escrow = mk();
assert_eq!(escrow.state(), EscrowState::Created);
// confirm_milestone exige Active.
assert_eq!(escrow.confirm_milestone(0), Err(Error::InvalidState));
// raise_dispute exige Active.
assert_eq!(escrow.raise_dispute(), Err(Error::InvalidState));
// resolve_dispute exige Disputed.
assert_eq!(
escrow.resolve_dispute(Resolution::SplitFifty),
Err(Error::InvalidState)
);
// El estado permanece Created — no hubo mutación ni transfer.
assert_eq!(escrow.state(), EscrowState::Created);
}
}
}