Skip to content

chore: bring develop into audit-fixes - #359

Open
Aman035 wants to merge 55 commits into
developfrom
merge/develop-into-audit-fixes
Open

chore: bring develop into audit-fixes#359
Aman035 wants to merge 55 commits into
developfrom
merge/develop-into-audit-fixes

Conversation

@Aman035

@Aman035 Aman035 commented Sep 1, 2026

Copy link
Copy Markdown
Member

Brings develop into the audit-fixes work so it can land. 31 conflicts resolved, full suite green.

Every audit fix is preserved. Verified two ways: all 34 finding ids that carried a code marker still do, and of 6001 distinctive lines the audit branch added, every absence is accounted for by a deliberate resolution below (renamed types, dropped unused params, replaced tests, gofmt).

The rename

develop moved universalClient/chains/ to externalchains/ and renamed common.UniversalTx to InboundObservation and common.OutboundEvent to OutboundObservation. Our work moved onto the new paths and names; the stale chains/ tree is gone.

Conflicts where neither side was correct

  • svm/tx_builder.go: develop prepends a recipient ATA create; the gateway now creates it on every path and meters the rent, so prepending makes the gateway see it already present and the rent falls outside gas_used, reopening F-2026-18815. The prepend is gone, and revert and rescue carry the two account slots the gateway needs to do the create. This matches fix: F-2026-18815 | follow the gateway recipient ATA creation on SVM revert and rescue #361, now merged.
  • create_outbound.go: develop resolves the rescue asset with GetTokenConfigByPRC20(event.PRC20), trusting caller-supplied identity. The rescued amount is always the original inbound's, so that pairs one asset's amount with another's identity. Our derivation and cross-check are folded into develop's PRC20 branch; its PC20 branch is kept.
  • svm/event_parser.go: develop's readGasUsed leaves gas_used at zero on a short payload, which reports a full refund. Our strict per-type offset and length check is kept, and develop's wrapper_address extraction added for Pc20WrapperAddress.
  • scripts/test_node.sh: took develop's denom_metadata (cosmos/evm v0.5+ needs it) but not its precompile list, which reintroduced the legacy 0x..ca and 0x..cb addresses that F-2026-18829 exists to remove, and dropped our usigverifier 0xEC..01.
  • evm.go: develop's moduleSenderNonce does a plain get-then-increment with no reconciliation against the account nonce, which is the desync F-2026-18189 reported. Its call sites now route through nextModuleSenderNonce and burnModuleSenderNonce.

Other notable resolutions

  • chains.go: kept our removeStaleChains extraction. Removing inline under the read lock deadlocks, since sync.RWMutex is not reentrant. Dropped ensurePushChain, which develop deleted (the push chain is managed by core now).
  • types.pb.go and types.pulsar.go regenerated from the merged proto rather than hand-merged.
  • go.mod and go.sum: develop's cosmos-sdk 0.53.7 with the newer pushchain/evm pin from fix(uexecutor): bill reverted inbound payloads for gas used (F-2026-18824 rec 2) #360. The older fork returns a nil receipt on a reverted payload, which is what fix(uexecutor): bill reverted inbound payloads for gas used (F-2026-18824 rec 2) #360 bumped it for, so taking develop's pair wholesale silently disabled its gas billing.
  • svm/event_parser.go also drops the revert gas_used read, since the gateway removed that field. Develop's wrapper_address extraction for PC20 is kept.
  • Our TestConstructInbound_* tests ported onto develop's buildInboundObservation.
  • TSSFundMigrationAmount stays removed; the fund-migration test now asserts the pinned TransferAmount.

Verification

go build ./..., go vet ./... and go test -tags=test ./... all clean.

Aman035 and others added 30 commits July 9, 2026 16:32
…nting Under-Reports Outbound Cost (#304)

* fix: include OP-Stack L1 data fee in outbound GasFeeUsed accounting (F-2026-18145)

* refactor: read L2+L1 gas fee from a single receipt call (F-2026-18145)

* refactor: single GetReceipt method with GasFee helper, drop typed receipt fetch (F-2026-18145)

* refactor: keep GetTransactionReceipt name, move gas-fee helper out of rpc_client (F-2026-18145)

* fix: source gas price from tx, not receipt effectiveGasPrice, for fee accounting (F-2026-18145)

* fix: use receipt effectiveGasPrice for gas fee, guard missing field, drop tx fetch (F-2026-18145)

* test: add skipped live-RPC gas-fee check for Sepolia and Base Sepolia (F-2026-18145)

* fix: error instead of zero gas fee when receipt fee cannot be determined (F-2026-18145)
…rum Change Without Deletion or Retirement (#305)

* fix: delete keyshares superseded by quorum change or key refresh (F-2026-18190)

* fix: resolve keyshare pubkeys per share instead of paging full key history (F-2026-18190)

* refactor: rename keysharegc to keysharesweeper, sweep hourly (F-2026-18190)

* refactor: move keyshare sweeper into keyshare package, guard Start with sync.Once (F-2026-18190)

* refactor: sweep keyshares daily and run once at start (F-2026-18190)

* test: cover GetKeyByID and guard nil query response (F-2026-18190)

* refactor: drop redundant pending-process and fund-migration guards from keyshare sweeper (F-2026-18190)
…ze Outbound Withdrawals via Gap Nonce (#306)

* fix: bound coordinator-assigned nonce above finalized to block gap-nonce freeze (F-2026-18191)

* fix: anchor nonce ceiling to pending nonce so congestion cannot false-reject (F-2026-18191)

* test: cover nonceBounds pending fallback branches (F-2026-18191)
…ied Hash (Payload vs SigningHash Split) (#309)

* fix: bind verified signing hash to the DKLS setup message before ACK (F-2026-18199)

* fix: bind setup participants for keygen, keyrefresh and quorumchange too (F-2026-18199)

* refactor: consolidate setup binding into one entry point and one helper file (F-2026-18199)

* test: move setup decoder tests to utils_test and cover both symmetrically (F-2026-18199)

* test: end-to-end proof that a mismatched payload hash refuses session and emits no shares (F-2026-18199)

* fix: bind setup threshold by parsing the setup TLV, closing the downgrade gap (F-2026-18199)
…fied Hash (Payload vs SigningHash Split) (#312)

* test: pin quorumchange threshold encoding and accept u16 threshold (F-2026-18199)

* test: cover setup decoders across keygen, refresh, quorumchange and sign (F-2026-18199)
…es Unbacked Synthetic Minting (#308)

* fix: only accept solana gateway events emitted by the gateway program (F-2026-18198)

* test: end-to-end proof that forged solana gateway events are not stored (F-2026-18198)

* fix: detect truncated solana log buffer so dropped gateway events are not silent (F-2026-18198)

* fix: identify which solana gateway event was dropped, not just that logs were cut (F-2026-18198)

* chore: drop svm event observation doc from branch

* revert: drop instruction discriminator reads from svm event listener

* test: unbalanced solana invoke logs must not underflow the attribution stack (F-2026-18198)
…erflow Panics Can Kill puniversald (#322)

* fix: reject overflowing ABI offsets and contain decode panics in the evm log parser (F-2026-18804)

* chore: gofmt evm event parser

* fix: contain decode panics in the svm log parser too (F-2026-18804)

* Revert "fix: contain decode panics in the svm log parser too (F-2026-18804)"

This reverts commit 0eaf1a9.
…fecycle: Stale-Remove Deadlock and Unclosed DB Handles (#325)

* fix: remove stale chains outside the read lock and close per-chain databases (F-2026-18797)

* fix: apply the same database ownership rule to the push chain path (F-2026-18797)
…unks Stall Cursor With No Adaptive Shrink (#324)

* fix: shrink the log query span on rejection and commit partial block progress (F-2026-18802)

* test: assert exact block coverage across span shrinks and range boundaries (F-2026-18802)
…erflow StateDB Balance Enabling Native Mint and Drain (#315)

Rejects all three cosmos vesting MsgCreate* types at the top level, not just inside authz.MsgExec.
…nable Universal Executor Module Impersonation (#316)

* fix: F-2026-18200 | bind gasless signer to signing key and reject non-20-byte addresses

Ante now enforces pubKey.Address() == signer for new gasless accounts, and
GetAddressPair / MustConvertCosmosToHex reject anything that is not exactly
20 bytes instead of truncating onto a module address.

* fix: drop signature gas consumption from the gasless new-account path

Gasless txs skip fee deduction entirely, so charging gas has no economic
effect. The TxSigLimit cap alone bounds the multisig work; reverts the
NewAccountInitDecorator signature change and the SigGasConsumer wiring.
…VM Ante for MsgEthereumTx (#317)

* fix: remove x/group module and drop wasm stargate capability

Both are generic nested-message dispatchers that reach the message router
after the ante handler has run, letting an MsgEthereumTx skip the EVM ante
(F-2026-18197). Neither is used by Push. Adds a remove-group upgrade handler
that prunes the group store.

* chore: drop the remove-group upgrade handler

audit-fixes targets mainnet, which starts from a fresh genesis where x/group
is never mounted, so no store deletion is required. The StoreUpgrades handler
belongs on the testnet branch, where donut has an existing group store.
…ycle Gaps Strand Bridged Principal (#319)

* fix: attach outbounds on isCEA contract callback success (F-2026-18195)

Attach inside the callback CacheContext so a nested UniversalGatewayPC burn and its OutboundTx/PendingOutbounds rows commit atomically; a failed attach discards the cache and records a FAILED PcTx.

* test: cover isCEA contract callback outbound attach and rollback (F-2026-18195)
… 0x…00ca After Runtime Moved to 0xEC…01 (#320)

* fix: activate ed25519 verifier at 0xEC..01 in genesis and drop legacy 0x..00ca

Nothing is registered at 0x..00ca, so declaring it active panics the EVM
precompile lookup, and the real verifier was never activated.

* feat: add usigverifier-precompile-fix upgrade handler

Drops the legacy ed25519 verifier address from EVM ActiveStaticPrecompiles
and adds 0xEC..01 if missing, for chains already past genesis.

* chore: fix ed25519 precompile address in README and drop unused UEA_SVM_BYTECODE

* chore: drop usigverifier-precompile-fix upgrade handler

audit-fixes targets mainnet, which starts from a fresh genesis, so the
genesis scripts already carry the correct active_static_precompiles list
and there are no live params to migrate. The upgrade handler belongs on
the testnet branch instead, where donut has live params still declaring
the legacy 0x...00ca address.

Moves TestGenesisScriptsActivateCurrentVerifier out of the handler's
test file into precompiles/usigverifier so the genesis half of the fix
stays covered.

* fix: drop the stale utxhashverifier address from genesis lists

0x..00CB has no implementation anywhere and remove-utxverifier strips it from
live chains, so leaving it in genesis re-introduced it on every fresh chain.
Confirmed absent from live donut params, so this is script-only cleanup.
… Valid Outbounds After Committed UEA Burn (#323)

* fix: attach payload outbounds inside ExecutePayloadV2's cache (F-2026-18825)

The gateway burn was committed before the handlers attached the outbounds, so
one invalid leg of a multicall erased the valid ones with their burns already
final and the failure swallowed into RevertError.

* test: cover UEA multicall outbound atomicity (F-2026-18825)
…Sign Threshold, Not TssKeyHistory Shareholders (#326)

* fix(tss): select fund migration signers from the old key's shareholders

* fix(tss): validate fund migration participants against the old key too

* revert unrelated gofmt changes

* refactor(tss): take the threshold as a parameter instead of a second helper

* docs(tss): note the departure budget on an old key
…pe=0 While Claiming Funds → Mapped to GAS (#327)

* fix(chains): reject truncated UniversalTx events instead of defaulting tx_type to GAS

* fix(common): guard event cleaner lifecycle with a mutex and wait for its goroutine

* revert unrelated gofmt change in client_test

* test: fold new tests into the existing per-source test files
…s Current TSS Address After Key Rotation (#314)

* fix: check outbound nonce against the key that signed, not the current TSS (F-2026-18827)

* test: add the broadcaster rotation regression and restore a misplaced doc comment
…e to EVM-Only isNative Check (#307)

* fix: recognize Solana native marker in SVM builder isNative check (F-2026-18196)

* fix: also treat hex-encoded zero pubkey as native SOL (F-2026-18196)

* docs: record that core sends EVM zero hex for solana native, base58 on reverts (F-2026-18196)

* test: assert both native marker forms build identical accounts (F-2026-18196)

* docs: state why the hex length check is load bearing
…w Confirmation Counts and Prematurely Finalize Inbounds (#302)

* fix: guard confirmation depth against RPC height skew; gate zero-confirmation instant routes to testnet (F-2026-18139)

* chore: rename Network to PushNetwork, trim comments (F-2026-18139)

* refactor: extract default confirmation depths to common constants; EVM fast default 5 (F-2026-18139)

* chore: log RPC height skew at debug not warn (F-2026-18139)

* test: cover RPC height skew end to end; pin signature discovery to finalized

* feat: allow zero confirmations per chain for instant-finality sources

* revert per-chain instant finality opt-in; no such source chain today
The hardcoded literal decoded to 42 bytes, which F-2026-18200's signer-length guard rejects in GetAddressPair before ExecutePayload runs. Red on audit-fixes since #317.
…ive Balance and Is Raceable by Dust Inflows (#303)

* fix: pin fund-migration signing hash to coordinator amount, verify it against live balance (F-2026-18142)

* feat(utss): pin fund-migration transfer amount on chain

Admin supplies the observed balance; the keeper derives transfer_amount from
it using the gas figures it fetches, stores it on the migration and emits it,
so validators sign one pinned amount instead of re-deriving from a live
balance. Rejects a balance that cannot cover its own fee.

* test(utss): set bech32 prefixes before validating the signer

Without them validSigner fails to parse and ValidateBasic returns on the
signer check, so the balance assertions never ran.

* fix(push): carry the pinned transfer amount into the fund migration event

* fix(tss): sign the chain-pinned fund migration amount instead of a live balance

* chore(tss): drop PinnedMigrationAmount, unused since the amount left the wire

* test: end-to-end that the pinned migration amount reaches the wire

---------

Co-authored-by: Nilesh Gupta <guptanilesh2312@gmail.com>
…uccess-Vote Starves Oldest-1000 Pending Queue (#318)

* fix: paginate pending outbound polling so a stuck prefix cannot hide newer rows (F-2026-18817)

* fix: paginate chain config query so it cannot silently cap at the SDK default (F-2026-18817)

* fix: carry pending outbound cursor between polls so the page budget costs latency not coverage (F-2026-18817)

* fix: request the full pending outbound set; the server pages by offset and returns no next key (F-2026-18817)

* fix: raise grpc receive limit so a large pending set does not fail the whole poll (F-2026-18817)

* fix: alternate pending outbound sweep direction instead of raising transport limits (F-2026-18817)

* fix: read newest-first every poll, sweep the backlog only when it exceeds one request (F-2026-18817)

* simplify: read pending outbounds newest-first, drop the backlog sweep machinery (F-2026-18817)

* fix: walk pending outbounds oldest-first instead of reading one page
…h band (#329)

Only 64..88 base58 chars can decode to 64 bytes, so gating the decode on that
band is output-equivalent. Also cap tx_hash on the unauthenticated InboundKeys
query.
…es Unsignable INBOUND_REVERT and Blocks Rescue (#330)

* fix(uexecutor): abort unsignable inbound reverts instead of queueing them

buildRevertOutbound failed open: when the gas metadata lookup failed it
returned a PENDING outbound with empty gas fields, which attachOutboundsToUtx
indexed into PendingOutbounds unconditionally. UVs refuse to sign it, so the
row sat there forever, and non-CEA rescue was gated on a REVERTED
inbound-revert so the user had no way out either.

- buildRevertOutbound returns (outbound, error)
- on gas-metadata failure the revert is marked ABORTED with an AbortReason
- attachOutboundsToUtx indexes only PENDING outbounds, and emits
  outbound_aborted for the rest
- the non-CEA rescue gate accepts REVERTED or ABORTED

* test(uexecutor): cover aborted inbound-revert and rescue recovery

- keeper unit tests drive buildRevertOutbound with the gas lookup mocked both
  ways: resolvable stays PENDING with exact gas fields and is indexed,
  unresolvable aborts with a reason and is not
- integration tests assert the revert is ABORTED, absent from PendingOutbounds,
  and that a non-CEA RESCUE_FUNDS is then accepted
- fix the unit fixture's auth store key (authtypes.StoreKey != ModuleName) and
  wire the real account keeper so UniversalCore calls work
An empty inner message list passed the allowlist loop vacuously, making the tx gasless and skipping the fee and min-gas-price decorators.
0xNilesh and others added 24 commits August 26, 2026 19:41
…te Unregistered ChainMetas Keys (#333)

* fix: reject MsgVoteChainMeta for unregistered chains (F-2026-18803)

Gate Keeper.VoteChainMeta on uregistry before any state read/write, and cap
observed_chain_id length + CAIP-2 shape in ValidateBasic.

* test: cover the unregistered-chain chain-meta gate (F-2026-18803)

Assert no ChainMetas row is written, at keeper and integration level.
REJECTED is a quorum asserting the observation is invalid, not a stuck
deposit, so refunding it would pay out against a deposit the validator set
said never happened. Adds an alarm if the unreachable state ever occurs.
Length-cap before parse, then BitLen<=256, via a shared helper applied to the six UniversalPayload numeric fields, Inbound.Amount and OutboundTx.Amount.
TSS_KEY ballots use 100% of the DKLS participant set, not 2/3+1; recompute
would rewrite both the threshold and the eligible voters. Gate on an explicit
default-deny allow-list (INBOUND_TX, OUTBOUND_TX, FUND_MIGRATION).
…m in EndBlock (#337)

Removes the O(active-set) IAVL scan CreateBallot ran on every ballot creation.
x/tx compares fee payer to signer with a raw string compare, so an uppercase
bech32 alias of the victim slips past it. Nothing on Push signs with AUX.
…r in step (#342)

Route every module-sender DerivedEVMCall through one helper that reads the
module account's EVM nonce, burns one nonce per attempt, and writes both back.
…e event (#343)

Reject a rescue whose event PRC20 disagrees with the original asset's registered PRC20.
#344)

A PENDING ballot whose every eligible voter has already voted can never
receive another vote, so it is terminal in fact; the admin hatch now
opens for it. REJECTED stays refused (F-2026-18801).
…345)

Ids came straight off collections.Sequence, whose first value is 0, while
MsgVoteFundMigration.ValidateBasic rejects migration_id == 0 as "unset".
The first migration on a fresh chain was therefore unvotable, never left
PendingMigrations, and blocked every later migration for that chain.

Allocate as sequence + 1: stored ids start at 1 and 0 stays reserved for
"unset", so the ValidateBasic guard remains a real check.

F-2026-18789
)

verifyEd25519RawMessage charged a flat 4000 gas no matter how long the
message was, while ed25519.Verify hashes the whole slice (~58us at 32B,
~922us at 1MB). Charge 4000 + 12 per 32-byte word instead, matching the
SHA-256 precompile's per-word rate, and hard-cap the message at 128 KiB
since a view method can be looped from memory without re-paying calldata.

verifyEd25519 stays flat: it always verifies the 66-byte hex form of a
bytes32 digest, so its cost cannot vary with the calldata.
… Invalid Address and Strands Outbound (#341)

* fix(svm): resolve empty outbound recipient to the sender's CEA

Honours the gateway's bytes("") parking convention on withdraw instead of rejecting "0x" pre-sign, which stranded the outbound PENDING with the PRC20 already burned. F-2026-18184.

* docs(svm): trim the parking sentinel comments and drop an incorrect claim

* test(svm): pin pre-sentinel recipient behaviour and run the parked path on devnet

---------

Co-authored-by: aman035 <guptaaman200115@gmail.com>
…026-18823

The two F-2026-18147 tests were written before #330 landed and asserted the
revert outbound is PENDING and queued. #330 aborts a revert whose gas metadata
is unresolvable, which is what the harness's UniversalCore stub produces, so
they now assert ABORTED and not-queued, matching the sibling happy-path test.
…napshot an Oversized Ballot Quorum (#347)

Exclude jailed validators from GetEligibleVoters so a validator jailed in
BeginBlock is not snapshotted into ballots created later in the same block.
…s ChainMeta Vote Authority (#348)

Gate MsgVoteChainMeta on GetEligibleVoters (ACTIVE/PENDING_JOIN + bonded +
not tombstoned) so a removed, still-bonded validator cannot reinsert votes.
…nd Unbounded gRPC Response in Universal Validator (#349)

* fix: cap the universal payload at 128 KiB and bound the UV gRPC receive size

* test: keep the huge-decimal assertion honest under the payload size cap
* chore: adapt to cosmos/evm v0.6.0 and bump the pin

v0.6.0 removes cosmos/evm's custom x/ibc/transfer wrapper and adds stateDB /
callFromPrecompile to CallEVM. Cherry-picked from #272 with the pin retargeted
and one newer call site fixed; no upgrade handler (fresh-genesis branch).

* chore: re-point evm pin at merged audit-fixes and adapt revert-outbound mocks

Pin moves off the PR-branch commit onto evm audit-fixes (89c7e52b), which
carries v0.6.0 plus every merged audit fix. build_revert_outbound_test.go
arrived after this branch was opened and mocked the pre-v0.6.0 CallEVM
signature: v0.6.0 inserts stateDB, so the mock needed NewStateDB and the
method-name matcher moved to index 8.
…(F-2026-18144) (#350)

Fee-paying txs are self-limiting: the ante handler requires
ceil(minGasPrice * gasLimit), so an absurd gas limit costs absurd money.
Gasless txs pay nothing, so nothing bounded the gas they declared, while that
declared gas was still added to the fee market's cumulative gas wanted for the
block. Under max_gas: -1 the per-tx block-limit check is inert, so two gasless
txs each declaring MaxInt64 sum past what EndBlock can convert to int64 - an
error that surfaces through FinalizeBlock after the block is decided.

New uexecutor param max_gasless_tx_gas (default 100,000,000), enforced by
GaslessGasLimitDecorator on the existing txpolicy.IsGaslessTx predicate,
before NewGasWantedDecorator accumulates the declaration. A parameter and not
a constant because "too low" stops the universal validators voting and must be
fixable by proposal in minutes. Zero is rejected at genesis and on update, and
an unreadable or unset parameter falls back to the default rather than to
"no cap".

universalClient/pushsigner declares 100,000,000 instead of 500,000,000. Live
donut data: gas_wanted median/max 500,000,000 against a gas_used max of
5,011,877 - the largest real consumer used 1.0024% of what it declared.
…dator for vote msgs (#351)

F-2026-18186 rec 3: a fresh key could send a gasless vote, get its account
committed by the ante cache, and have the message fail afterwards, leaving the
row behind. Reject the five validator-only vote msgs before any account is
written.
…uctions (#357)

* fix(svm): read gateway events from emit_cpi inner instructions

* refactor(svm): read the shared outbound fields at fixed offsets

* refactor(svm): drop the redundant signature-level failure check

* test(svm): pin gateway attribution across lookup-table account segments

* docs(svm): trim the event observation comments
…8824 rec 2) (#360)

* fix(uexecutor): bill reverted inbound payloads for gas used

Inbound routes only; charge clamped to balance and never fails the inbound.

* chore: pin evm to 7ac130d5 (DerivedEVMCall response propagation)

* chore: trim comments
…revert and rescue (#361)

* fix(svm): follow the gateway's recipient ATA creation on revert and rescue

* fix(svm): report the revert gas fee from the gateway's reimbursement event

* Revert "fix(svm): report the revert gas fee from the gateway's reimbursement event"

This reverts commit 2565b87.

* docs(svm): drop the stale instruction-order comment and trim the rest

* test(svm): pin that a revert votes zero gas rather than empty

* test: drop the vote validation test
@Aman035 Aman035 changed the title merge: bring develop into audit-fixes chore: bring develop into audit-fixes Sep 2, 2026
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