Skip to content

fix: dev_mint_index audit#743

Open
charmful0x wants to merge 2 commits intoimpr/secure-actionsfrom
audit/mint-index
Open

fix: dev_mint_index audit#743
charmful0x wants to merge 2 commits intoimpr/secure-actionsfrom
audit/mint-index

Conversation

@charmful0x
Copy link

@charmful0x charmful0x commented Mar 10, 2026

findings

dev_mint_index:parse_signal/3 double negation

in dev_mint_index:parse_signal the module was doing {ok, <<"withdraw">>} -> {ok, -RawQuantity}; however there is one important caveat - full pot -> outbox -> mint-index stack:

  • dev_pot:undelegate/6 calls dev_pot:send_delegation_notice/6 with -Amount
  • dev_pot:send_delegation_notice/6 pass that -ve quantity as is to dev_process_outbox:send/3
  • dev_process_outbox:notify/3 fwd the original notice fields to subscribers as x-* fields without changing the sign, as is
  • dev_mint_index:notify/3 receive that forwarded withdraw notification and calls parse_signal/3
  • parse_signal/3 then negated the already-negative RawQuantity again, turning a withdrawal delta into a +ve value

impact: undelegations increase the internal indexed-deposits/<address> model instead of decreasing it

dev_mint_index:must_redelegate/4 inverted check

the must_redelegate/4 function was checking UpdateEvery >= ChangesSinceUpdate. which means the function will always return true for any positive UpdateEvery and reset ChangesSinceUpdate to 0, so it would always redelegate on every change.

there are no specific tests for this fn or doc spec, but i assumed it means "redelegate every N changes".

question:

    UpdateEvery = hb_ao:get(<<"update-every">>, Base, 0, Opts),
    ChangesSinceUpdate = hb_ao:get(<<"changes-since-update">>, Base, 0, Opts),

should UpdateEvery default to 1 instead of 0, so it doesnt redelegate always at defaults?

clarification needed

re redelegate/3 and indexed-deposits:

  • redelegate/3: right now redelegate/3 only emits a redelegation event and resets changes-since-update. what is the intended implementation of redelegate/3 here? is it currently incomplete/missing?

  • deposit model: the current model assumes indexed-deposits/<address> to track indexed deposits. is the model here assuming the following stack? :
    1. AO: dev_pot (root pot) + dev_token
    2. FLPs: child pots
    3. PI: a 'special' child/index pot that mirrors AO-side delegation preference on the root AO pot

asking because update_model/4 ignores resource, so it is currently assuming a single-resource parent model

@charmful0x charmful0x added question Further information is requested and removed question Further information is requested labels Mar 11, 2026
@charmful0x charmful0x changed the title wip: dev_mint_index audit fix: dev_mint_index audit Mar 11, 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.

1 participant