Spike/hot near counterparty#412
Conversation
📝 WalkthroughHOT Cross-Chain Bridge: Stellar ↔ NEAR — Security Review (updated)OverviewSpike implementing a HOT-first Stellar↔NEAR bridge proof-of-concept:
Top critical review items (must inspect)
Medium-priority checks
Positive findings
Smart-contract vulnerability patterns to cross-check(See kadenzipfel/smart-contract-vulnerabilities)
Minimal actionable recommendations (for reviewers / maintainers)
WalkthroughAdds NEAR vault-counterparty, Soroban HOT bridge adapter, and a HOT relayer service plus workspace wiring, build/deploy scripts, monitoring helpers, tests, and documentation to implement HOT-based Stellar↔NEAR cross-chain flows. ChangesNEAR Counterparty Bridge Contract
Soroban HOT Bridge Adapter Contract
HOT Relayer Service
Cross-Chain Bridge Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
1823c3b to
9fb66f0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2b7d57ccf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.call_omni( | ||
| "mt_transfer", | ||
| json!({ |
There was a problem hiding this comment.
Use transfer_call when forwarding market supply
When the counterparty forwards assets into a Templar market, a plain mt_transfer only moves the NEP-245 balance to the market contract; the market credits supply exclusively from mt_on_transfer after mt_transfer_call carries the DepositMsg::Supply message (contract/market/src/impl_token_receiver.rs). In the curator flow described by this crate, forward_to_market will therefore leave the counterparty with no supply position and the transferred assets unusable by the withdrawal helpers; send mt_transfer_call with the serialized "Supply" msg and enough gas instead.
Useful? React with 👍 / 👎.
b1c5dd2 to
07c404a
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contract/vault-counterparty/justfile`:
- Around line 196-199: The monitor-withdrawal just task is an unreadable
single-line with embedded Node/Python; extract it into a standalone script and
call it from just. Create a new executable script (e.g.,
scripts/monitor-withdrawal.sh) that accepts nonce, receiver, interval arguments,
sets safe shell flags (set -euo pipefail), implements the existing loop and all
embedded Python/Node calls (preserving HOT_STELLAR_ENCODED_RECEIVER, ONCE
behavior and headers/args), and then change the justfile target
monitor-withdrawal to invoke that script (bash
contract/vault-counterparty/scripts/monitor-withdrawal.sh "{{nonce}}"
"{{receiver}}" "{{interval}}") while keeping the check-monitor-tools dependency
and semantics.
In `@contract/vault-counterparty/src/lib.rs`:
- Around line 386-427: In mt_on_transfer, move the guard checks so the contract
calls self.assert_not_paused() and self.assert_withdraw_gas_budget() before
mutating state with self.increase_supply_position(...); keep existing amount
validation (Self::assert_amount) but reorder to perform the pause and gas budget
assertions immediately after that validation and before increase_supply_position
(and then proceed to start_operation, hot_withdraw_promise, and the
on_operation_complete callback as before).
In `@contract/vault/soroban/hot-bridge-adapter/src/lib.rs`:
- Around line 413-418: Add an inline comment on the surplus calculation around
the lines that compute `surplus = balance.checked_sub(reserved)...` explaining
that `reserved` is produced by `record_returned` and should always be <=
`balance`, so an underflow here is not a normal user path but indicates an
external transfer or accounting bug (i.e., clarify that the `checked_sub` guard
is defensive and documents why it would fail). Reference the variables
`balance`, `reserved`, `surplus` and the `record_returned` call in the comment
for auditors.
- Around line 137-139: The supply path currently rejects redeploying any pending
returned balance by checking returned_of(&env, &asset) and returning
AdapterError::PendingReturnedBalance; add a short in-contract comment above the
supply handler (reference: the supply function/path, the returned_of(&env,
&asset) check, and AdapterError::PendingReturnedBalance) explaining this
operational constraint, and also add a one-line note to the contract README
describing that supply cannot be used to redeploy assets while there is a
pending returned balance to avoid confusion for operators and reviewers.
In `@service/funding-bridge/scripts/hot_stellar_contract_deposit.sh`:
- Line 34: The script currently builds SENDER_ACCOUNT via the local alias
(SENDER_ACCOUNT="$(stellar keys address templar-hot-mainnet)") but later signs
with STELLAR_SECRET_KEY, risking mismatched identity; make the sender identity a
single source of truth by deriving both public and secret values from the same
source: either compute SENDER_ACCOUNT from the STELLAR_SECRET_KEY (use the
stellar keys address operation on the secret) or derive STELLAR_SECRET_KEY from
the templar-hot-mainnet key and then use that secret for signing; update all
uses (SENDER_ACCOUNT, STELLAR_SECRET_KEY and any sender_id) so they come from
that one canonical assignment and remove the inconsistent alternative.
- Around line 41-46: The recomputation of RECEIVER_HEX in
hot_stellar_contract_deposit.sh using TARGET_NEAR_ACCOUNT is brittle and caused
mismatched bytes; change the script to use a proven default receiver hex unless
an explicit override is provided: keep the RECEIVER_HEX variable but first check
for an env var (e.g., RECEIVER_HEX_OVERRIDE) and use it if present, otherwise
fall back to the stored constant proven receiver hex; ensure the python
recompute block (that references TARGET_NEAR_ACCOUNT) is removed or guarded so
it runs only when an explicit flag (e.g., RECOMPUTE_RECEIVER=true) is set, and
update any references to RECEIVER_HEX accordingly.
In `@service/funding-bridge/scripts/monitor_hot_withdrawal.sh`:
- Around line 24-50: The ENCODED_RECEIVER generation block currently hardcodes
root='/data/projects/t-private-fork' and swallows all exceptions by printing ''
which lets the script continue as healthy; change the python stub called by
ENCODED_RECEIVER to (1) derive the repo/deps path from an environment variable
(e.g., REPO_ROOT) or fail if not found instead of using the hardcoded path, (2)
on any exception print a descriptive error to stderr and exit non‑zero so
ENCODED_RECEIVER is empty/invalid only on success, and (3) in the shell script
check ENCODED_RECEIVER after the python call and if empty or non‑zero exit the
script (or log error and exit) so the NEAR withdrawal query (the block around
lines 98-109) does not get skipped under a broken setup. Ensure references to
ENCODED_RECEIVER and the python invocation are updated accordingly.
In `@service/hot-relayer/Dockerfile`:
- Around line 24-25: Add a Docker HEALTHCHECK so orchestrators can detect a hung
hot-relayer process: add a HEALTHCHECK instruction to the Dockerfile (near the
EXPOSE 3001 / ENTRYPOINT ["/usr/local/bin/hot-relayer"] block) that probes the
service on port 3001 (e.g., an HTTP liveness path like /health or /ping) and
returns non-zero on failure, with sensible interval, timeout and retry settings
to avoid false positives; ensure the command targets localhost:3001 and document
the chosen health endpoint in the repo.
In `@service/hot-relayer/scripts/complete_hot_deposit.sh`:
- Around line 81-90: The config check in check_required_config currently
validates HOT_RELAYER_STELLAR_RECEIVER but the script and docs expect
HOT_RELAYER_NEAR_RECEIVER; update the validation to check the correct
environment variable(s): replace the HOT_RELAYER_STELLAR_RECEIVER check with a
check for HOT_RELAYER_NEAR_RECEIVER (or add both checks if both receivers are
supported), and ensure the error message and print_error text reference the
matching variable name used elsewhere in the script (check_required_config,
HOT_MPC_API_URL, HOT_RELAYER_NEAR_RECEIVER).
In `@service/hot-relayer/src/config.rs`:
- Around line 164-168: The current join(&self, path: &str) method panics on join
errors; change its signature to return a Result<reqwest::Url, url::ParseError>
(or the concrete error type returned by reqwest::Url::join) and replace
unwrap_or_else with self.0.join(path.trim_start_matches('/')).map_err(|e| e) (or
map to a custom error type if you prefer); then update all callers of join to
handle the Result (propagate with ? or handle the error) so runtime code no
longer panics. Ensure you reference the join method and adjust any callers
accordingly.
- Around line 199-201: The HOT_RELAYER_AUTH_TOKEN constructor currently accepts
any non-empty string; update the pub(crate) fn new(value: &str) in config.rs to
enforce a stronger policy: after calling
require_non_empty("HOT_RELAYER_AUTH_TOKEN", value)? validate the trimmed token
meets a minimum length (suggest >= 32) and a charset complexity rule (e.g.,
contains at least two/three of: lowercase, uppercase, digits, symbols, or match
a provided regex) and return an appropriate ConfigError on failure (reuse
ConfigError variants or add an InvalidToken/Validation error). Keep
require_non_empty, perform checks on the result (Self(trimmed.to_string()) only
on success), and reference the new/require_non_empty/ConfigError symbols when
implementing the change.
In `@service/hot-relayer/src/hot_relayer.rs`:
- Around line 423-425: After deserializing into SignatureResponse, validate that
parsed.signature is not empty and return an appropriate HotRelayerError if it
is; modify the code in hot_relayer.rs immediately after the
serde_json::from_slice call (around SignatureResponse and parsed.signature) to
check parsed.signature.is_empty() and map that case to a clear error (e.g., add
HotRelayerError::EmptySignature or reuse an existing variant like
HotRelayerError::Decode/InvalidSignature) instead of returning
Ok(parsed.signature) when empty.
- Around line 25-68: The DTOs (PendingWithdrawal, PendingWithdrawData,
StellarDepositEvent, DepositSignRequest, StellarWithdrawExecution) currently use
raw String for nonce, amount, token_id and receiver fields which allows invalid
states; introduce small newtype wrappers (e.g., Nonce, Amount, TokenId,
AccountId) that implement TryFrom<String> and TryFrom<&str> plus serde support
(Deserialize via try_from / custom deserializer or
#[serde(try_from)]/#[serde(transparent)] as appropriate) to validate/normalize
at boundary deserialization, then replace the String-typed fields in those
structs with the newtypes and update any callers/constructors to use the TryFrom
conversions so parsing happens once at ingress and core code receives
strongly-typed, validated values.
In `@service/hot-relayer/src/routes.rs`:
- Around line 103-107: The handlers complete_deposit and complete_withdrawal
currently perform Json(req) extraction before calling require_auth(&state,
&headers), causing malformed/unauthenticated requests to fail during body
deserialization; change each handler to call require_auth(&state, &headers)
first (e.g., accept State(state) and HeaderMap headers, call
require_auth(&state, &headers) immediately) and only then deserialize the body —
either by swapping extraction order or by accepting raw Bytes (or
axum::body::Bytes) and calling serde_json::from_slice after authentication;
ensure you reference the same function names (complete_deposit,
complete_withdrawal) and require_auth so the logic is applied consistently
across both endpoints.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fd23bd4d-3b24-44b8-92f0-7323e0d29c09
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockcontract/vault/soroban/hot-bridge-adapter/tests/fixtures/hot_stellar_locker_mainnet_cc74acb56fd01ef560b5d443cd58fe11d5acbece0e4001612ebb31020fc92f97.wasmis excluded by!**/*.wasm
📒 Files selected for processing (31)
Cargo.tomlcontract/vault-counterparty/.env.examplecontract/vault-counterparty/Cargo.tomlcontract/vault-counterparty/README.mdcontract/vault-counterparty/justfilecontract/vault-counterparty/scripts/load-stellar-secret-env.shcontract/vault-counterparty/src/lib.rscontract/vault/justfilecontract/vault/soroban/.env.examplecontract/vault/soroban/README.mdcontract/vault/soroban/hot-bridge-adapter/Cargo.tomlcontract/vault/soroban/hot-bridge-adapter/src/lib.rscontract/vault/soroban/hot-bridge-adapter/tests/mainnet_locker_wasm.rscontract/vault/soroban/justfileplans/CROSS_CHAIN_BRIDGE.mdservice/funding-bridge/README.mdservice/funding-bridge/scripts/hot_stellar_contract_deposit.shservice/funding-bridge/scripts/monitor_hot_withdrawal.shservice/hot-relayer/.env.exampleservice/hot-relayer/Cargo.tomlservice/hot-relayer/Dockerfileservice/hot-relayer/README.mdservice/hot-relayer/docs/operations.mdservice/hot-relayer/scripts/complete_hot_deposit.shservice/hot-relayer/src/bridge_transport.rsservice/hot-relayer/src/config.rsservice/hot-relayer/src/hot_relayer.rsservice/hot-relayer/src/lib.rsservice/hot-relayer/src/main.rsservice/hot-relayer/src/metrics.rsservice/hot-relayer/src/routes.rs
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
contract/vault-counterparty/src/lib.rs (3)
215-252:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
new()is missing an invariant already enforced on updates.
validate_config_updaterejectsnear_market == env::current_account_id(), but initialization does not. That allows a fresh deployment to enter a self-referential config that the update path later forbids. Reuse the same validation fromnew()so init and rotation enforce identical routing invariants.As per coding guidelines, keep the Rust codebase DRY and make invalid states unrepresentable; this invariant currently exists only on the update path.
Also applies to: 832-841
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contract/vault-counterparty/src/lib.rs` around lines 215 - 252, The initializer new() allows near_market to equal env::current_account_id(), but validate_config_update later rejects that; fix by enforcing the same invariant in new(): before constructing Self (in contract/vault-counterparty src lib.rs inside new()), call or reuse validate_config_update logic (or at minimum assert near_market != env::current_account_id()) so initialization and updates share identical routing invariants; apply the same change to the equivalent initializer/rotation site referenced around the 832-841 area so both places prevent self-referential near_market.Source: Coding guidelines
183-188:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRetries can be rerouted after config rotation.
Operationpersists onlykindandamount, butretry_tracked_operationrebuilds the outbound call from the currentnear_market,omni_contract,omni_token_id,stellar_receiver, andintents_integration. If an owner accepts a config or intents update after an operation fails, retrying that operation replays it against the new route, not the original one. For bridge flows, that is an asset-loss footgun. Either snapshot the resolved routing fields intoOperationor block config/integration changes while any operation is nonterminal.As per coding guidelines, “make invalid states unrepresentable” for protocol state and request payloads; storing only
kindandamountleaves an invalid retry route representable.Also applies to: 562-607, 741-776
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contract/vault-counterparty/src/lib.rs` around lines 183 - 188, Operation currently only stores id/kind/amount/status causing retries in retry_tracked_operation to rebuild outbound calls from mutable config (near_market, omni_contract, omni_token_id, stellar_receiver, intents_integration) so retries can be routed to a new, incorrect target; fix by snapshotting the resolved routing fields into Operation and using them on retry: add explicit fields to the Operation struct (e.g., near_market_snapshot, omni_contract_snapshot, omni_token_id_snapshot, stellar_receiver_snapshot, intents_integration_snapshot), update (de)serialization and constructors that create Operation to populate these snapshots from the resolved route, and change retry_tracked_operation to use these snapshot fields instead of reading current config; alternatively (if snapshotting is unacceptable) implement a blocker: disallow config/intents updates when any Operation is nonterminal by checking operations for nonterminal statuses before applying updates and returning an error, and update places mentioned (retry_tracked_operation and the code paths that create Operation instances) to match the chosen approach.Source: Coding guidelines
209-210:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
withdrawal_requestsis not preserving queue order.
BTreeMapsorts byAccountId, so Line 348 executes the lexicographically smallest requester, not the earliest requester. Once multiple suppliers have pending exits, this breaks the advertised queue semantics and can reorder or starve withdrawals. Model pending requests with an ordered queue (for example, a sequence id plus lookup map, or aVecDeque) instead of key order by account.As per coding guidelines, use stronger types when ordering semantics matter and “make invalid states unrepresentable”; a map here permits an invalid execution order.
Also applies to: 323-335, 348-352
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contract/vault-counterparty/src/lib.rs` around lines 209 - 210, The current use of BTreeMap for withdrawal_requests preserves AccountId sort order, causing withdrawals to be processed by lexicographic account rather than FIFO; change the data model so queue order is explicit (e.g., replace withdrawal_requests: BTreeMap<AccountId, u128> with an ordered queue plus lookup—such as a VecDeque of request entries (struct with seq_id, account, amount) and a secondary HashMap<AccountId, seq_id/amount> for fast lookup/removal, or add a monotonically increasing sequence id stored with each request and keep a Vec/VecDeque ordered by seq_id). Update the code that enqueues requests, dequeues/executes withdrawals (the loop that iterates withdrawal_requests and currently picks the smallest AccountId), and any removal/lookup logic to use the new queue + map structures (preserve supply_positions usage), ensuring FIFO semantics and preventing reordering or starvation.Source: Coding guidelines
contract/vault-counterparty/justfile (1)
126-153:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
receiver=is only cosmetic here.Line 128 lets operators override
receiver, but Line 150 still sendsHOT_STELLAR_RECEIVER_HEX. That meansjust stellar-deposit receiver=...can log one NEAR receiver while the deposit is actually minted to whichever HOT receiver hex is in the environment. For a mainnet flow, either remove thereceiveroverride entirely or fail unless the provided receiver is known to match the configured hex.Suggested simplification
-stellar-deposit amount='1000000' asset='native' receiver='': check-stellar-tools +stellar-deposit amount='1000000' asset='native': check-stellar-tools : "${HOT_STELLAR_RECEIVER_HEX:?set HOT_STELLAR_RECEIVER_HEX}" - RECEIVER="{{receiver}}"; if [ -z "$RECEIVER" ]; then RECEIVER="${COUNTERPARTY_ACCOUNT:?set COUNTERPARTY_ACCOUNT or pass receiver}"; fi + RECEIVER="${COUNTERPARTY_ACCOUNT:?set COUNTERPARTY_ACCOUNT}"Based on learnings from the PR objectives,
hot_deposit_receiver_hexis supposed to come from verified HOT receiver bytes rather than local recomputation, so exposing a separatereceiveroverride without binding it to those bytes is unsafe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contract/vault-counterparty/justfile` around lines 126 - 153, The `stellar-deposit` recipe lets users pass receiver=... but still always uses HOT_STELLAR_RECEIVER_HEX when invoking deposit, causing a mismatch; update the recipe (stellar-deposit) to either remove the cosmetic receiver override or validate it: if a `receiver` arg is provided, compute its verified HOT receiver hex and compare to HOT_STELLAR_RECEIVER_HEX (or fail the run) and only proceed if they match; ensure references include HOT_STELLAR_RECEIVER_HEX, receiver, and the deposit invoke that uses HOT_STELLAR_LOCKER_CONTRACT so the invocation uses the verified hex (or errors out) rather than silently logging a different NEAR receiver.service/hot-relayer/src/hot_relayer.rs (2)
287-290: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winKeep
HotNonceacross the signer boundary.The newtypes close the serde boundary, but
withdraw_sign(&str)re-opens it immediately and lets callers/implementations reintroduce invalid nonces into the core signing path. Take&HotNoncehere and serialize to&stronly insideHotMpcApiClient.♻️ Suggested direction
#[async_trait] pub trait HotMpcSigner { - async fn withdraw_sign(&self, nonce: &str) -> Result<String, HotRelayerError>; + async fn withdraw_sign(&self, nonce: &HotNonce) -> Result<String, HotRelayerError>; async fn deposit_sign(&self, request: &DepositSignRequest) -> Result<String, HotRelayerError>; } @@ async fn plan_stellar_withdraw_execution_unchecked<S: HotMpcSigner + Sync>( signer: &S, pending: &PendingWithdrawal, ) -> Result<StellarWithdrawExecution, HotRelayerError> { - let signature = signer.withdraw_sign(pending.nonce.as_str()).await?; + let signature = signer.withdraw_sign(&pending.nonce).await?; Ok(build_stellar_withdraw_execution(pending, signature)) }As per coding guidelines, prefer parsing over validation and make invalid states unrepresentable at API boundaries.
Also applies to: 592-597
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@service/hot-relayer/src/hot_relayer.rs` around lines 287 - 290, Change the HotMpcSigner API to accept a typed HotNonce instead of &str: update the trait method signature withdraw_sign(&self, nonce: &HotNonce) -> Result<String, HotRelayerError>, then update all implementations and call sites (including HotMpcApiClient) to pass &HotNonce and perform any serde/string conversion only inside HotMpcApiClient where the transport boundary exists; ensure compilation by adjusting impls that previously accepted &str and parsing/serializing there so invalid nonces cannot cross the signer boundary.Source: Coding guidelines
515-536:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftBound HOT MPC response size before buffering it.
response.bytes().awaitreads the full body into memory. A misconfigured or degraded HOT MPC endpoint can return arbitrarily large error pages here and pin memory on the relayer request path. This client only expects a tiny JSON{ "signature": ... }, so stream with a hard cap and fail once the cap is exceeded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@service/hot-relayer/src/hot_relayer.rs` around lines 515 - 536, Replace the unbounded response.bytes().await with a bounded stream-read that enforces a max size (e.g. define MAX_MPC_RESPONSE_BYTES) and returns HotRelayerError::Http if the cap is exceeded or the stream errors; check response.content_length() first and error early if it exceeds the cap, otherwise iterate response.bytes_stream(), accumulate chunks into a Vec<u8> up to the cap, error if adding a chunk would overflow the cap, then use the collected bytes for serde_json::from_slice to produce SignatureResponse and keep the existing HotRelayerError::Decode handling and parsed.signature empty check; update references to response.bytes(), SignatureResponse, parsed.signature, and HotRelayerError::Http accordingly.service/hot-relayer/scripts/complete_hot_deposit.sh (1)
38-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop requiring dead config, or wire it into the request.
The script hard-requires
HOT_MPC_API_URLandHOT_RELAYER_TOKEN_ID, but it actually talks toSERVICE_URLand always sends argvTOKEN_ID. That rejects documented valid setups while still not preventing a mismatched token from being posted. Either remove those checks, or default/validate the request fields from the env values you require.♻️ Suggested direction
-check_required_config() { - if [ -z "${HOT_MPC_API_URL:-}" ]; then - print_error "HOT_MPC_API_URL is required in .env" - exit 1 - fi - +check_required_config() { if [ -z "${HOT_RELAYER_NEAR_RECEIVER:-}" ]; then print_error "HOT_RELAYER_NEAR_RECEIVER is required in .env" exit 1 @@ - if [ -z "${HOT_RELAYER_TOKEN_ID:-}" ]; then - print_error "HOT_RELAYER_TOKEN_ID is required in .env" - exit 1 - fi - if [ -z "${HOT_RELAYER_AUTH_TOKEN:-}" ]; then print_error "HOT_RELAYER_AUTH_TOKEN is required in .env" exit 1 fi } @@ -TOKEN_ID="${3:-}" +TOKEN_ID="${3:-${HOT_RELAYER_TOKEN_ID:-}}"Also applies to: 81-99, 103-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@service/hot-relayer/scripts/complete_hot_deposit.sh` around lines 38 - 44, The script currently enforces HOT_MPC_API_URL and HOT_RELAYER_TOKEN_ID even though it posts to SERVICE_URL and sends the TOKEN_ID argv; remove the dead strict checks or make the env vars authoritative: either (A) remove checks for HOT_MPC_API_URL and HOT_RELAYER_TOKEN_ID and rely only on SERVICE_URL and the passed TOKEN_ID, or (B) default the request fields from the env (set TOKEN_ID="${1:-$HOT_RELAYER_TOKEN_ID}" and API_URL="${SERVICE_URL:-$HOT_MPC_API_URL}") and add a validation step that errors if a provided argv TOKEN_ID conflicts with HOT_RELAYER_TOKEN_ID; update all occurrences of HOT_MPC_API_URL, HOT_RELAYER_TOKEN_ID, SERVICE_URL and TOKEN_ID in the script to follow the chosen approach so documented setups work and mismatched tokens are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contract/vault-counterparty/scripts/monitor-withdrawal.sh`:
- Around line 21-25: Wrap each inline Node probe (the two node -e invocations
that run the hot withdraw/sign and hot clear_completed_withdrawal probes) in an
explicit async IIFE so top-level await is not relied on and execution is
deterministic across Node setups; ensure any global APIs used (fetch) are
available (e.g., require('node:fetch') or use globalThis.fetch if polyfilled)
inside the IIFE, and call the IIFE immediately, replacing the current
top-level-await usage in both the withdraw/sign and clear_completed_withdrawal
snippets.
In `@script/prebuild-test-contracts.sh`:
- Line 5: The script sets NEAR_RUST_TOOLCHAIN default to 1.86.0 which conflicts
with the repo pin (rust-toolchain.toml) and CI (RUSTUP_TOOLCHAIN=1.89.0); update
the NEAR_RUST_TOOLCHAIN default in prebuild-test-contracts.sh from "1.86.0" to
"1.89.0" (the RUST toolchain version pinned in rust-toolchain.toml) so that when
the script sets RUSTUP_TOOLCHAIN="$NEAR_RUST_TOOLCHAIN" it no longer overrides
the repo/CI pin with an older version.
---
Outside diff comments:
In `@contract/vault-counterparty/justfile`:
- Around line 126-153: The `stellar-deposit` recipe lets users pass receiver=...
but still always uses HOT_STELLAR_RECEIVER_HEX when invoking deposit, causing a
mismatch; update the recipe (stellar-deposit) to either remove the cosmetic
receiver override or validate it: if a `receiver` arg is provided, compute its
verified HOT receiver hex and compare to HOT_STELLAR_RECEIVER_HEX (or fail the
run) and only proceed if they match; ensure references include
HOT_STELLAR_RECEIVER_HEX, receiver, and the deposit invoke that uses
HOT_STELLAR_LOCKER_CONTRACT so the invocation uses the verified hex (or errors
out) rather than silently logging a different NEAR receiver.
In `@contract/vault-counterparty/src/lib.rs`:
- Around line 215-252: The initializer new() allows near_market to equal
env::current_account_id(), but validate_config_update later rejects that; fix by
enforcing the same invariant in new(): before constructing Self (in
contract/vault-counterparty src lib.rs inside new()), call or reuse
validate_config_update logic (or at minimum assert near_market !=
env::current_account_id()) so initialization and updates share identical routing
invariants; apply the same change to the equivalent initializer/rotation site
referenced around the 832-841 area so both places prevent self-referential
near_market.
- Around line 183-188: Operation currently only stores id/kind/amount/status
causing retries in retry_tracked_operation to rebuild outbound calls from
mutable config (near_market, omni_contract, omni_token_id, stellar_receiver,
intents_integration) so retries can be routed to a new, incorrect target; fix by
snapshotting the resolved routing fields into Operation and using them on retry:
add explicit fields to the Operation struct (e.g., near_market_snapshot,
omni_contract_snapshot, omni_token_id_snapshot, stellar_receiver_snapshot,
intents_integration_snapshot), update (de)serialization and constructors that
create Operation to populate these snapshots from the resolved route, and change
retry_tracked_operation to use these snapshot fields instead of reading current
config; alternatively (if snapshotting is unacceptable) implement a blocker:
disallow config/intents updates when any Operation is nonterminal by checking
operations for nonterminal statuses before applying updates and returning an
error, and update places mentioned (retry_tracked_operation and the code paths
that create Operation instances) to match the chosen approach.
- Around line 209-210: The current use of BTreeMap for withdrawal_requests
preserves AccountId sort order, causing withdrawals to be processed by
lexicographic account rather than FIFO; change the data model so queue order is
explicit (e.g., replace withdrawal_requests: BTreeMap<AccountId, u128> with an
ordered queue plus lookup—such as a VecDeque of request entries (struct with
seq_id, account, amount) and a secondary HashMap<AccountId, seq_id/amount> for
fast lookup/removal, or add a monotonically increasing sequence id stored with
each request and keep a Vec/VecDeque ordered by seq_id). Update the code that
enqueues requests, dequeues/executes withdrawals (the loop that iterates
withdrawal_requests and currently picks the smallest AccountId), and any
removal/lookup logic to use the new queue + map structures (preserve
supply_positions usage), ensuring FIFO semantics and preventing reordering or
starvation.
In `@service/hot-relayer/scripts/complete_hot_deposit.sh`:
- Around line 38-44: The script currently enforces HOT_MPC_API_URL and
HOT_RELAYER_TOKEN_ID even though it posts to SERVICE_URL and sends the TOKEN_ID
argv; remove the dead strict checks or make the env vars authoritative: either
(A) remove checks for HOT_MPC_API_URL and HOT_RELAYER_TOKEN_ID and rely only on
SERVICE_URL and the passed TOKEN_ID, or (B) default the request fields from the
env (set TOKEN_ID="${1:-$HOT_RELAYER_TOKEN_ID}" and
API_URL="${SERVICE_URL:-$HOT_MPC_API_URL}") and add a validation step that
errors if a provided argv TOKEN_ID conflicts with HOT_RELAYER_TOKEN_ID; update
all occurrences of HOT_MPC_API_URL, HOT_RELAYER_TOKEN_ID, SERVICE_URL and
TOKEN_ID in the script to follow the chosen approach so documented setups work
and mismatched tokens are detected.
In `@service/hot-relayer/src/hot_relayer.rs`:
- Around line 287-290: Change the HotMpcSigner API to accept a typed HotNonce
instead of &str: update the trait method signature withdraw_sign(&self, nonce:
&HotNonce) -> Result<String, HotRelayerError>, then update all implementations
and call sites (including HotMpcApiClient) to pass &HotNonce and perform any
serde/string conversion only inside HotMpcApiClient where the transport boundary
exists; ensure compilation by adjusting impls that previously accepted &str and
parsing/serializing there so invalid nonces cannot cross the signer boundary.
- Around line 515-536: Replace the unbounded response.bytes().await with a
bounded stream-read that enforces a max size (e.g. define
MAX_MPC_RESPONSE_BYTES) and returns HotRelayerError::Http if the cap is exceeded
or the stream errors; check response.content_length() first and error early if
it exceeds the cap, otherwise iterate response.bytes_stream(), accumulate chunks
into a Vec<u8> up to the cap, error if adding a chunk would overflow the cap,
then use the collected bytes for serde_json::from_slice to produce
SignatureResponse and keep the existing HotRelayerError::Decode handling and
parsed.signature empty check; update references to response.bytes(),
SignatureResponse, parsed.signature, and HotRelayerError::Http accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8276c1a7-96f9-4033-b81d-4555f1d8de2f
📒 Files selected for processing (17)
.github/workflows/test.ymlcontract/vault-counterparty/justfilecontract/vault-counterparty/scripts/monitor-withdrawal.shcontract/vault-counterparty/src/lib.rscontract/vault/soroban/hot-bridge-adapter/README.mdcontract/vault/soroban/hot-bridge-adapter/src/lib.rscontract/vault/soroban/hot-bridge-adapter/tests/mainnet_locker_wasm.rsrust-toolchain.tomlscript/prebuild-test-contracts.shservice/hot-relayer/Dockerfileservice/hot-relayer/README.mdservice/hot-relayer/scripts/complete_hot_deposit.shservice/hot-relayer/src/bridge_transport.rsservice/hot-relayer/src/config.rsservice/hot-relayer/src/hot_relayer.rsservice/hot-relayer/src/main.rsservice/hot-relayer/src/routes.rs
| set -ex | ||
|
|
||
| ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd .. && pwd)" | ||
| NEAR_RUST_TOOLCHAIN="${NEAR_RUST_TOOLCHAIN:-1.86.0}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Pinned root toolchain:"
sed -n '1,20p' rust-toolchain.toml | sed -n 's/.*channel\s*=\s*"\([^"]*\)".*/\1/p'
echo "Prebuild default toolchain:"
sed -n '1,40p' script/prebuild-test-contracts.sh | sed -n 's/.*NEAR_RUST_TOOLCHAIN="\${NEAR_RUST_TOOLCHAIN:-\([^}]*\)}".*/\1/p'
echo "All explicit RUSTUP_TOOLCHAIN usages in CI/scripts:"
rg -n 'RUSTUP_TOOLCHAIN|NEAR_RUST_TOOLCHAIN' .github/workflows script rust-toolchain.tomlRepository: Templar-Protocol/contracts
Length of output: 531
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Search for NEAR_RUST_TOOLCHAIN overrides/usages:"
rg -n "NEAR_RUST_TOOLCHAIN" . --hidden || true
echo
echo "Who calls/sources prebuild-test-contracts.sh?"
rg -n "prebuild-test-contracts\.sh" script .github/workflows --hidden || true
echo
echo "Inspect script/test.sh call chain (relevant sections only):"
sed -n '1,220p' script/test.sh
echo
echo "Inspect script/gas-report.sh call chain (relevant sections only):"
sed -n '1,260p' script/gas-report.sh
echo
echo "Inspect full prebuild-test-contracts.sh (line 1-140):"
sed -n '1,140p' script/prebuild-test-contracts.shRepository: Templar-Protocol/contracts
Length of output: 2777
Align prebuild-test-contracts.sh default NEAR toolchain with repo pin
script/prebuild-test-contracts.sh defaults NEAR_RUST_TOOLCHAIN to 1.86.0 (line 5), but the repo pins rust-toolchain.toml to 1.89.0 and CI sets RUSTUP_TOOLCHAIN: 1.89.0. The script then forces RUSTUP_TOOLCHAIN="$NEAR_RUST_TOOLCHAIN", so the CI pin is ignored and generated prebuilt artifacts can be produced with 1.86.0.
Suggested fix
-NEAR_RUST_TOOLCHAIN="${NEAR_RUST_TOOLCHAIN:-1.86.0}"
+NEAR_RUST_TOOLCHAIN="${NEAR_RUST_TOOLCHAIN:-1.89.0}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| NEAR_RUST_TOOLCHAIN="${NEAR_RUST_TOOLCHAIN:-1.86.0}" | |
| NEAR_RUST_TOOLCHAIN="${NEAR_RUST_TOOLCHAIN:-1.89.0}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@script/prebuild-test-contracts.sh` at line 5, The script sets
NEAR_RUST_TOOLCHAIN default to 1.86.0 which conflicts with the repo pin
(rust-toolchain.toml) and CI (RUSTUP_TOOLCHAIN=1.89.0); update the
NEAR_RUST_TOOLCHAIN default in prebuild-test-contracts.sh from "1.86.0" to
"1.89.0" (the RUST toolchain version pinned in rust-toolchain.toml) so that when
the script sets RUSTUP_TOOLCHAIN="$NEAR_RUST_TOOLCHAIN" it no longer overrides
the repo/CI pin with an older version.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
service/funding-bridge/scripts/monitor_hot_withdrawal.sh (1)
26-72:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winShell syntax error: command substitution closed before heredoc content.
The
)"; thenon line 26 prematurely closes the command substitution before the heredoc content (lines 27-68). Line 69's)"; thenis then orphaned. ShellCheck confirms the parse failure (SC1073, SC1072).The script will not execute at all in its current form.
Proposed fix
-if ! ENCODED_RECEIVER="$(python3 - <<'PY' "$RECEIVER" "$ENCODER_WORKSPACE")"; then +if ! ENCODED_RECEIVER="$(python3 - "$RECEIVER" "$ENCODER_WORKSPACE" <<'PY' import sys addr = sys.argv[1] root_arg = sys.argv[2] ... PY -)"; then +)"; then echo "Failed to derive ENCODED_RECEIVER; set HOT_ENCODER_WORKSPACE or fix local encoder deps." >&2 exit 1 fiMove the positional arguments before the heredoc operator and remove the premature closing on line 26.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@service/funding-bridge/scripts/monitor_hot_withdrawal.sh` around lines 26 - 72, The heredoc invocation is malformed: the positional args were placed before the heredoc terminator causing the command substitution to close early; fix the python3 invocation that produces ENCODED_RECEIVER by moving the arguments ("$RECEIVER" "$ENCODER_WORKSPACE") into the python3 command (i.e. pass them immediately after python3, not before the <<'PY' terminator) so the heredoc body is consumed as the script, remove the premature closing `)"; then` and ensure the if ! ENCODED_RECEIVER="$(python3 ... <<'PY' ... PY )"; then structure is a single, valid command substitution that preserves the heredoc block and still exits with the intended error handling.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@service/funding-bridge/scripts/monitor_hot_withdrawal.sh`:
- Around line 26-72: The heredoc invocation is malformed: the positional args
were placed before the heredoc terminator causing the command substitution to
close early; fix the python3 invocation that produces ENCODED_RECEIVER by moving
the arguments ("$RECEIVER" "$ENCODER_WORKSPACE") into the python3 command (i.e.
pass them immediately after python3, not before the <<'PY' terminator) so the
heredoc body is consumed as the script, remove the premature closing `)"; then`
and ensure the if ! ENCODED_RECEIVER="$(python3 ... <<'PY' ... PY )"; then
structure is a single, valid command substitution that preserves the heredoc
block and still exits with the intended error handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 89adb632-2480-4128-8577-aa58c76011af
📒 Files selected for processing (5)
contract/vault-counterparty/scripts/monitor-withdrawal.shcontract/vault/soroban/hot-bridge-adapter/src/lib.rsscript/prebuild-test-contracts.shservice/funding-bridge/scripts/hot_stellar_contract_deposit.shservice/funding-bridge/scripts/monitor_hot_withdrawal.sh
Cross-Chain Bridge: Stellar <-> NEAR
NOTE: this is a spike outcome draft. We would productionise this later. We would also implement the stellar market adapter.
Contract Boundaries
stellar_receivernear_marketomni_token_idomni_contractRelayer Boundary
Introduce a bridge-transport interface in the relayer service:
BridgeRelayer::complete_deposit(event)BridgeRelayer::complete_withdrawal(pending)HotBridgeRelayeris the first implementation.If transport changes later, implement a new relayer adapter behind the same trait and keep the loop/orchestration code stable.
What Changes on Transport Swap
omni_contract.What Does Not Change
Operational Notes
carrion256.near: Stellar deposit routing is currently stable across repeated requests, returning the same deposit addressGDJ4JZXZELZD737NVFORH4PSSQDWFDZTKW3AIDKHYQG23ZXBPDGGQBJKand memo49425867on consecutive calls.carrion256.near, while preserving the option to re-query if HOT changes routing behavior later.carrion256.near, reuse the first proven-good receiver bytes instead of recomputing during the spike:52fd581de41f4bace88c936b89bf267a1161426a466adc518cd9e56f201651dd6axVRkdWZFh81dbLC4bBCqHqWhsD7r4u7Rv52D4vshbAprocess_bridge_depositwithCant sign by HOT VerifyVerified Mainnet Transaction Trail
4dbb21cbed36c30eb7620d70524f7d3acf73eee71ed443fc4fde61cc537dbd1fGCMVV45LOZUYYVXOQJ626VXGL3KFXY73DHFBT4EDPDBE2LN4USRQDYVVtoGD3SOHKDS7CDGDOTJKP6VNAOEXC3Y5BRWD3WIEK65ZQAJUMTBGE4TVBZcarrion256.tg:6hrJEufLstUX3KuHsCUaU9YYMYCtZnTpSQpLzNLBaeoMabf8a4e3650dd797a8bce5cb46b22fa791473f6fb17c5e6e4584e301cb8fffd6tocarrion256.tgCkAQwwt3vrfcbuPqXQ2mpDqvVaAVdDZUKyXtJWGNY5wmcarrion256.nearcreated on mainnetcarrion256.nearfor bootstrap:2i6U5G2ZFSDYSYncKck39uqGRkrpxVu9rLyzyLkdh8Hacarrion256.nearcarrion256.nearwith correctedcargo nearartifact:6FWCW3ErGaH2DmgsKFigGkMAXSc6f182mWaLP5eiRtyrAnjCXStSps8Exe6VjkhY5NnjhQK9ZutMVdKke2MuTbswstellar_receiver,near_market,omni_token_id,omni_contract,owner, andcuratorixlm-ixlmusdc.v1.tmplr.near:storage_balance_of("ixlm-ixlmusdc.v1.tmplr.near")returned present on-chaincarrion256.near:storage_balance_of("carrion256.near")returned present on-chain56876328c88bd52755fa4cc3346442e9bace1adf3e962c57d5a0cddb719b9615CCLWL5NYSV2WJQ3VBU44AMDHEVKEPA45N2QP2LL62O3JVKPGWWAQUVAGCAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA1000000stroops (0.1 XLM)GCMVV45LOZUYYVXOQJ626VXGL3KFXY73DHFBT4EDPDBE2LN4USRQDYVV17761737702736281190002TTZViJT3zBHkt1LaoADJZf56ouS9yBZcaZUqa3xmaQw0here.tgv2_1.omni.hot.tgminted1100_111bzQBB5v7AhLyPMDwS8uJgQV24KaAPXtwyVWu2KXbbfQU6NXRCzv2_1.omni.hot.tgtransferred that amount tointents.nearintents.nearmintednep245:v2_1.omni.hot.tg:1100_111bzQBB5v7AhLyPMDwS8uJgQV24KaAPXtwyVWu2KXbbfQU6NXRCztocarrion256.nearcarrion256.near:1000000Withdrawal Notes
carrion256.nearto HOT failed because the counterparty passed the raw StellarG...address into HOTwithdraw(...).ScVal(Address)before calling HOTnep245:v2_1.omni.hot.tg:...) into HOTwithdraw(...).Failed to parse chain ID 'nep245:v2'AGhfPGre4pcpgxAEchV4cMEtsMCSThSXLjJHno2BUuFV9Sct9sHHWK3VA6Qr9z4qgT1bSxTztecnY3VPh1fLg3cXwithdraw_to_stellarfrom the counterparty still failed withNot enough balance.intents.nearas a NEP-245 wrapped token, not as a raw balance directly owned bycarrion256.nearonv2_1.omni.hot.tgintents.near mt_balance_of(carrion256.near, nep245:...) = 1000000v2_1.omni.hot.tg mt_balance_of(carrion256.near, 1100_...) = 0intents.near mt_withdrawwith the raw HOT token contract + token id:FZixtw3Sr97TWD7dnmDJApX5yGCXEbXTdzbB1fsGEenxtoken = "v2_1.omni.hot.tg"token_ids = ["1100_111bzQBB5v7AhLyPMDwS8uJgQV24KaAPXtwyVWu2KXbbfQU6NXRCz"]receiver_id = "bridge-refuel.hot.tg"msg.receiver_id = "1114wxgAxsZMgcipUZ92Tv8iTpTqiyZMoQqC9kWRbj8jVURaZcnBS1uQCiL"intents.nearburned500000ofnep245:v2_1.omni.hot.tg:1100_111bzQBB5v7AhLyPMDwS8uJgQV24KaAPXtwyVWu2KXbbfQU6NXRCzfromcarrion256.nearv2_1.omni.hot.tgtransferred500000of raw omni token1100_111bzQBB5v7AhLyPMDwS8uJgQV24KaAPXtwyVWu2KXbbfQU6NXRCztobridge-refuel.hot.tgv2_1.omni.hot.tgemittedmt_burnfrombridge-refuel.hot.tgwith memo / withdrawal nonce1776177129000001087571rpc1.hotdao.ai/withdraw/signandrpc2.hotdao.ai/withdraw/signboth return404 {"detail":"Nonce not found or vas already cleared"}for nonce1776177129000001087571clear_completed_withdrawalreturnsfalsev2_1.omni.hot.tg.get_withdrawals_by_receiver(...)returns[]This change is