feat(precompiles): add scoped module authorizations - #3893
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3893 +/- ##
==========================================
- Coverage 59.45% 58.47% -0.99%
==========================================
Files 2319 2226 -93
Lines 198379 188327 -10052
==========================================
- Hits 117946 110123 -7823
+ Misses 69235 67758 -1477
+ Partials 11198 10446 -752
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PR SummaryHigh Risk Overview Staking exposes grant/revoke and delegate, redelegate, and undelegate “with authorization,” using three Distribution adds withdrawal grants for delegator rewards and validator commission (not set withdraw address), with authorized withdraw methods. Slashing adds unjail grant/revoke and Consensus: Solidity interfaces, ABIs, and end-to-end tests cover each module’s authorization flow. Reviewed by Cursor Bugbot for commit bf5c4cc. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Well-structured centralization of native authz grant/exec/revoke behind shared helpers, with the granter correctly derived from message signers and delegatecall/staticcall guarded on every new method. One confirmed correctness bug (authorized commission withdrawal measures the wrong account, so its EVM event reports 0 for validators with a custom withdraw address), plus a silent scope widening of the existing grantVoteAuthorization API.
Findings: 1 blocking | 9 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
grantStakingAuthorizationissuesGenericAuthorizationfor MsgDelegate/MsgBeginRedelegate/MsgUndelegate rather than CosmosStakeAuthorization, so there is no amount cap and no validator allow/deny list: until expiry the grantee can move or unbond the granter's entire stake. Funds stay with the granter (undelegated principal and rewards go to the delegator's own accounts), so this is griefing rather than theft, but it is worth stating explicitly in the Solidity NatSpec sinceStakeAuthorizationis the narrower primitive the SDK offers.- Payment asymmetry deserves a louder warning in the Solidity docs: for
delegateWithAuthorizationandsubmitProposalWithAuthorization,HandlePaymentUseicredits the granter, so the grantee funds a delegation and a proposal deposit they can never recover — unbonded principal and any deposit refund accrue to the granter. An integrating contract that naively forwardsmsg.valueloses it permanently. - Test coverage is happy-path only. Missing: expiration at or before block time (the new centralized check in
GrantGenericAuthorizationsis untested for slashing/staking/distribution), an unassociated grantee/delegator argument, executing a*WithAuthorizationmethod with no grant ever created (only the post-revoke case is covered),RevokeAuthorizationsreturning ErrNotFound when the whole group is absent, and a validator with a custom withdraw address — that last one is what would have caught the commission bug above. The partial-group revoke tolerance is only exercised in gov. - No EVM events are emitted for grant or revoke on any of the four precompiles, so the authorization lifecycle is invisible to EVM indexers while the use of an authorization does emit events. Pre-existing for
grantVoteAuthorization, but this PR multiplies the surface. - Per
AGENTS.md("Godocs say what a thing is, not why it came to be or how it works inside"), several new doc comments are rationale rather than description and should move to inline comments at the line that needs them:GrantGenericAuthorizations("Cosmos authz keys grants by message type, so..."),RevokeAuthorizations("Missing members are ignored so..."),withdrawDelegationRewardsFor,prepareSubmitProposal("so the two entry points cannot drift"), anddelegateFor("because a later association cannot safely merge..."). - Minor behavior change worth knowing about: in
withdrawDelegationRewardsandwithdrawValidatorCommission, argument validation and Sei-address resolution now sit outside therecover()guard (they used to be inside it). An out-of-gas panic from theGetSeiAddressstore read will now propagate instead of being converted to a precompile error. That actually matches the intent documented inRunAndCalculateGas("an executor that later exhausts its gas keeps its normal (propagating) out-of-gas semantics"), so I read it as fine — but it is a semantic change in an app-hash-breaking PR and should be a deliberate one. - Second-opinion passes: Codex reported "No material findings in the PR diff."
cursor-review.mdis empty — that pass produced no output, so treat this review and Codex's as the only coverage. - 2 suggestion(s)/nit(s) flagged inline on specific lines.
| return nil, 0, err | ||
| } | ||
| execute := func() (sdk.Int, error) { | ||
| balanceBefore := p.bankBalance(ctx, validator) |
There was a problem hiding this comment.
[blocker] This measures the balance delta on the validator operator's account address, but WithdrawValidatorCommission credits the operator's withdraw address:
// sei-cosmos/x/distribution/keeper/keeper.go:137-139
accAddr := sdk.AccAddress(valAddr)
withdrawAddr := k.GetDelegatorWithdrawAddr(ctx, accAddr)
err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, withdrawAddr, commission)So whenever the operator has called setWithdrawAddress, the delta here is 0 and the ValidatorCommissionWithdrawn event is emitted with amount = 0 even though a non-zero commission moved. The direct path (withdrawValidatorCommission) reports the real amount from the keeper return value, and the sibling withdrawDelegationRewardsWithAuthorization resolves the withdraw address correctly (distribution.go:393) — this path is the odd one out.
Since MsgWithdrawValidatorCommissionResponse is empty in this SDK version, the balance-delta approach is necessary; it just needs the right account:
execute := func() (sdk.Int, error) {
withdrawAddress := p.distrKeeper.GetDelegatorWithdrawAddr(ctx, validator)
balanceBefore := p.bankBalance(ctx, withdrawAddress)
...
return p.bankBalance(ctx, withdrawAddress).Sub(balanceBefore), nil
}Worth fixing before merge: this precompile version gets frozen into a legacy/vX snapshot at the next release cut, so the wrong event data becomes permanent for that height range. TestWithdrawAuthorizationFlow misses it because the default withdraw address is the account itself — a case with setWithdrawAddress would cover it.
| grantee, | ||
| expiration, | ||
| &govtypes.MsgVote{}, | ||
| &govtypes.MsgSubmitProposal{}, |
There was a problem hiding this comment.
[suggestion] This widens an already-deployed method: grantVoteAuthorization previously conferred MsgVote only, and now also confers MsgSubmitProposal. Existing on-chain callers (e.g. a protocol that delegates voting to a manager) will, after this upgrade, hand that manager the ability to submit arbitrary governance proposals in the granter's name — and there is no longer any way to grant vote-only.
Financially it is contained: prepareSubmitProposal funds the deposit from the grantee's msg.value via HandlePaymentUsei, so the granter is credited then debited and nets zero. But with msg.value == 0 the deposit is empty coins and a grantee can spam zero-deposit proposals attributed to the granter at nothing but gas cost — that is a real identity/reputation risk for the granter.
A separate grantProposalAuthorization / revokeProposalAuthorization pair would keep each EVM-facing permission matched to its name and preserve the existing method's meaning. If bundling is deliberate, please call the widened authority out in the PR description as a breaking semantic change to a shipped interface, not only in the NatSpec — the same applies to revokeVoteAuthorization at line 294, which now also drops the proposal grant.
| granter, | ||
| grantee, | ||
| expiration, | ||
| &stakingtypes.MsgDelegate{}, |
There was a problem hiding this comment.
[nit] The grouped message list is duplicated verbatim between grantStakingAuthorization here and revokeStakingAuthorization (line 319). The two must stay in sync or a granted message type silently becomes unrevocable — exactly the drift RevokeAuthorizations' missing-member tolerance is papering over.
A single package-level definition would make the group the invariant rather than the convention:
// stakingAuthorizationMsgs is the message-type group behind the EVM-facing
// staking permission.
var stakingAuthorizationMsgs = []sdk.Msg{
&stakingtypes.MsgDelegate{},
&stakingtypes.MsgBeginRedelegate{},
&stakingtypes.MsgUndelegate{},
}Same shape applies to the gov, slashing, and distribution groups.
There was a problem hiding this comment.
Well-structured refactor that centralizes native authz grant/exec/revoke behavior and adds scoped staking, slashing, distribution, and governance authorizations with end-to-end tests. One blocking concern: bundling MsgSubmitProposal into the governance grant as a plain GenericAuthorization gives the grantee an unbounded, burnable claim on the granter's balance outside the EVM entry point.
Findings: 1 blocking | 10 non-blocking | 4 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- Cursor's review file (
cursor-review.md) is empty — that pass produced no output. Only Codex's single finding was available as a second opinion. - I disagree with Codex's finding that renaming
grantVoteAuthorization/revokeVoteAuthorizationbreaks deployed contracts. Those selectors exist only in the unreleased top-level precompile: no snapshot underprecompiles/gov/legacy/*contains them, andprecompiles/gov/setup.gomaps the top-level executor tolatestUpgrade(registered versions stop at v6.5). A repo-wide grep also shows no stale references to the old names. Renaming pre-release is safe; adding legacy aliases would permanently enshrine a name the chain never shipped. - Test gaps: no test exercises grant expiry (calling a
*WithAuthorizationmethod afterexpirationhas passed) in any module; there is no negative test forsubmitProposalWithAuthorizationwithout/after a grant (the post-revoke assertion only coversvoteWithAuthorization); and only gov tests rejection of a past expiration — staking, slashing, and distribution grants don't. - The grouped message sets are defined three different ways: a package-level
governanceAuthorizationMsgsvar in gov, but inline variadic literals repeated at both the grant and revoke call sites in staking, distribution, and slashing. Per AGENTS.md's "guard at the choke point" rule, each module's permission→message-set mapping should be named once so grant and revoke provably cover the same set. grant*Authorizationoverwrites, andrevoke*Authorizationdeletes, any native grant of the same message type regardless of how it was created (SaveGrantis an unconditionalstore.Set). A grant a user established via a normal Cosmos tx will be silently re-expired or removed by the EVM call. Worth stating in the Solidity NatSpec.precompiles/distribution/distribution.gobankBalancehardcodessdk.DefaultBondDenom, while the staking precompile reads the configured base denom (sdk.MustGetBaseDenom()/evmKeeper.GetBaseDenom(ctx)). It matches the rest of distribution.go today, but the two conventions should converge.- For
submitProposalWithAuthorization, the deposit is refunded to the depositor of record — the proposer/granter — not the grantee whosemsg.valuefunded it. The NatSpec says "The caller supplies the initial deposit through msg.value" without noting the refund goes elsewhere; worth spelling out. - 3 suggestion(s)/nit(s) flagged inline on specific lines.
| // governance permission. | ||
| var governanceAuthorizationMsgs = []sdk.Msg{ | ||
| &govtypes.MsgVote{}, | ||
| &govtypes.MsgSubmitProposal{}, |
There was a problem hiding this comment.
[blocker] Bundling MsgSubmitProposal into the governance permission as a plain GenericAuthorization gives the grantee an unbounded, burnable claim on the granter's balance.
The EVM entry point is bounded — prepareSubmitProposal funds InitialDeposit from the grantee's msg.value via HandlePaymentUsei, so the granter is net-zero. But the grant written here is an ordinary native authz grant with no constraint beyond the message type. The same grantee can bypass the precompile entirely and broadcast a Cosmos MsgExec{MsgSubmitProposal{Proposer: <granter>, InitialDeposit: <any amount>}}; DispatchActions only checks GenericAuthorization.Accept on the type URL, and the deposit is then debited from the granter.
That deposit is not merely at risk of being locked — it is burned outright when the proposal never reaches MinDeposit during the deposit period (sei-cosmos/x/gov/abci.go:28 calls DeleteDeposits, "deletes all the deposits ... without refunding them") or when it is vetoed (abci.go:65). So a grantee can drain the granter by repeatedly submitting proposals that are left to expire.
The PR description anticipates the reputational side of this ("A grantee can submit proposals in the granter's name, including zero-deposit proposals") but not the financial one — a zero-deposit proposal is the harmless case. This also silently upgrades the trust model of a permission that was vote-only on main: a user delegating votes now hands over spend authority.
Suggested resolutions, in order of preference:
- Split proposal submission into its own
grantProposalAuthorization/revokeProposalAuthorizationpair so voting can be delegated without spend authority. - Keep the bundle but grant a constrained authorization for the submit leg rather than
GenericAuthorization. - If the bundle is intentional as-is, say so explicitly in the
@noticeongrantGovernanceAuthorizationand in the PR body: the grantee can spend and cause the permanent loss of arbitrary amounts of the granter's balance.
| if err == nil { | ||
| dstWithdrawnCoins, wErr := p.distributionKeeper.WithdrawDelegationRewards(ctx, delegator, dstValAddr) | ||
| if wErr == nil { | ||
| dstWithdrawnCoins, withdrawErr := p.distributionKeeper.WithdrawDelegationRewards(ctx, delegator, dstValAddr) |
There was a problem hiding this comment.
[suggestion] This pre-withdraw calls distributionKeeper.WithdrawDelegationRewards directly on the granter's behalf, outside authz. On the new redelegateWithAuthorization path that means a grantee holding only the staking grant (MsgDelegate/MsgBeginRedelegate/MsgUndelegate) triggers an action equivalent to MsgWithdrawDelegatorReward for the granter, which the staking grant does not cover — and which the separate grantWithdrawAuthorization exists precisely to gate.
There is no theft: the rewards land at the granter's configured withdraw address. But it is a real scope leak (and a forced realization event the granter did not delegate away). Either route the pre-withdraw through pcommon.ExecuteAuthorization as well, or document on redelegateWithAuthorization that the staking grant implies destination-validator reward withdrawal to the granter's own withdraw address.
| ) external returns (bool success); | ||
|
|
||
| /** | ||
| * @notice Authorize a grantee to delegate, redelegate, and undelegate on behalf of the caller |
There was a problem hiding this comment.
[suggestion] This @notice reads as if the grant is scoped to what the sibling EVM methods do, but the underlying grant is a GenericAuthorization per message type: unbounded amount, any validator, and — like the gov grant — usable through a raw Cosmos MsgExec rather than only through this precompile. delegateWithAuthorization is bounded by the grantee's msg.value, but the MsgUndelegate and MsgBeginRedelegate legs let a grantee move or unbond the granter's entire stake to any validator by either route.
Either document that explicitly here, or consider stakingtypes.StakeAuthorization, which natively supports validator allow/deny lists and a max amount and would make the "scoped" framing in the PR title accurate for staking.
| return msgServer.Exec(sdk.WrapSDKContext(ctx), &exec) | ||
| } | ||
|
|
||
| // RevokeAuthorizations removes every extant message-type grant in a shared |
There was a problem hiding this comment.
[nit] AGENTS.md is explicit that godocs state what a thing is, with rationale moved to an inline comment at the line that needs it, and that they never record design history. The second sentence here ("Missing members are ignored so grants created before a permission was expanded can still be revoked as a group") is both rationale and a reference to how the permission evolved.
Same pattern in GrantGenericAuthorizations ("Cosmos authz keys grants by message type, so ..."), withdrawDelegationRewardsFor, prepareSubmitProposal ("so the two entry points cannot drift"), and delegateFor. The rationale is genuinely load-bearing in each case — per the guide it should move to an inline comment at the tolerant continue / the shared step, not be dropped.
There was a problem hiding this comment.
The refactor into shared precompiles/common/authorization.go helpers is clean and the authz plumbing (grant/exec/revoke, MsgExecResponse decoding, ErrNotFound tolerance) is correct against sei-cosmos. The blocking concern is scope: grantStakingAuthorization mints unbounded native GenericAuthorizations for delegate/redelegate/undelegate, so a grantee can bypass the precompile via a native MsgExec and move the granter's entire stake and liquid balance — a risk the PR explicitly warns about for governance but not at all for staking.
Findings: 1 blocking | 8 non-blocking | 4 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
cursor-review.mdis empty — the Cursor pass produced no output, so this review merges only Claude's and Codex's findings.- I disagree with Codex's second finding ("does not provide the documented grouped
grantGovernanceAuthorization/revokeGovernanceAuthorizationinterface"). No such grouped method exists inGov.sol, the ABI, or the PR description — the PR explicitly specifies split vote/proposal grants, andTestVoteAndProposalAuthorizationFlowasserts that split. Not a defect. - Test-coverage gaps in the new shared helpers: (a) the
expiration <= ctx.BlockTime()guard moved out of gov intocommon.GrantGenericAuthorizationsand now governs four precompiles, but no test exercises a rejected expiration on the staking/distribution/slashing paths; (b) the grouped-revokeerrors.Is(err, sdkerrors.ErrNotFound) → continuebranch is never hit (all three staking grants exist at revoke time), so the "tolerates missing members" behavior the doc comment promises is unverified; (c) no test pins the intended scope of a staking/withdraw grant — i.e. that the grant does what the EVM method implies and nothing more. - Behaviour-changing edits are mixed into what is otherwise a readability refactor of existing methods.
delegate/redelegate/undelegatenow callmsg.ValidateBasic()before dispatching to the keeper, which they did not previously, so some inputs (e.g. non-positive amounts) now revert with a different error from a different layer on the pre-existing direct path.AGENTS.mdstates behaviour never changes in a readability refactor and that the proof is existing tests passing unchanged — please confirm the pre-existing staking/distribution tests were not edited to accommodate this. delegateFor's doc comment (precompiles/staking/staking.go:359) states "The delegator must be explicitly associated because a later association cannot safely merge a delegation created under its cast address" — butdelegateFordoes not enforce that; both callers do it independently viaGetSeiAddressByEvmAddress/GetSeiAddressFromArg. PerAGENTS.md("Guard at the choke point, never at each caller"), either move the check intodelegateForor move the rationale to the two entry points that actually hold the invariant.- 3 suggestion(s)/nit(s) flagged inline on specific lines.
Comments that couldn't be anchored to the diff
precompiles/gov/Gov.sol:124-- [nit] Now that governance authority is split in two,revokeVoteAuthorizationleaves anyMsgSubmitProposalgrant intact (andrevokeProposalAuthorizationleaves the vote grant intact) —TestVoteAndProposalAuthorizationFlowasserts exactly that. The doc is technically accurate, but a caller who granted both and reaches for "revoke" will plausibly assume it clears all governance authority. Worth stating explicitly here that each revoke is scoped to its own capability and both must be called.
| } | ||
| expiration := time.Unix(args[1].(int64), 0).UTC() | ||
|
|
||
| if err := pcommon.GrantGenericAuthorizations( |
There was a problem hiding this comment.
[blocker] Unbounded native grant behind a bounded-looking EVM method.
These are plain GenericAuthorizations keyed only by message type. The EVM entry points bound what the grantee can do (delegateWithAuthorization delegates exactly the grantee's own msg.value), but the grant itself does not — a grantee can skip the precompile entirely and submit a native Cosmos MsgExec:
MsgDelegatefor any amount out of the granter's own liquid balance, to any validator — locking it for the unbonding period;MsgBeginRedelegatemoving the granter's entire existing stake to a validator the grantee controls (e.g. 100% commission), or to one about to be slashed;MsgUndelegateof everything.
The PR recognises exactly this escalation class for governance and documents it prominently in Gov.sol; staking gets the same escalation with no warning anywhere, and Staking.sol's delegateWithAuthorization doc ("The caller supplies msg.value") actively implies the granter's own funds are not at risk.
sei-cosmos already ships the bounded primitive for this: stakingtypes.NewStakeAuthorization(allowed, denied, authzType, maxTokens) (sei-cosmos/x/staking/types/authz.go:19), which caps tokens and restricts the validator set per authorization type. That would make the native grant match what the EVM surface advertises.
At minimum, if the generic grant is a deliberate trade-off, mirror the Gov.sol treatment verbatim in Staking.sol — an explicit "grant only to a fully trusted account; the grantee can move your entire stake and liquid balance via a native Cosmos message" notice — so callers can't reach the wrong conclusion from the interface alone.
| * @return success Whether the authorization was successfully granted | ||
| * @notice This native MsgSubmitProposal authorization can also be used through Cosmos MsgExec with an arbitrary initial deposit debited from the caller. Grant it only to a fully trusted account; proposal deposits can be permanently lost | ||
| */ | ||
| function grantProposalAuthorization( |
There was a problem hiding this comment.
[suggestion] Agreeing with Codex's first finding on the substance: this is a native GenericAuthorization for MsgSubmitProposal, so a grantee can bypass submitProposalWithAuthorization with a native MsgExec and pick an arbitrary initial deposit debited from the granter — the deposit can be burned on a vetoed or failed proposal. Unlike the staking case, though, this one is disclosed in the PR body and in the @notice right below, so I'd treat it as an accepted trade-off rather than a defect.
One gap worth closing: the warning lives only in NatSpec, which is not carried in abi.json and so is invisible to anyone integrating from the ABI. Consider repeating it on grantProposalAuthorization in gov.go so it survives into the Go docs as well.
| return err | ||
| } | ||
| if _, err := msgServer.Revoke(sdk.WrapSDKContext(ctx), &revoke); err != nil { | ||
| if errors.Is(err, sdkerrors.ErrNotFound) { |
There was a problem hiding this comment.
[suggestion] This ErrNotFound → continue branch and the if !revoked fallthrough at line 90 are the load-bearing part of the "grouped revocation tolerates missing members" behaviour advertised in the doc comment, and neither is reachable from any test in this PR: every grouped revoke under test has all member grants present, and gov's single-message revoke never iterates.
A short case — grant staking authority, revoke MsgDelegate alone through the native authz msg server, then call revokeStakingAuthorization and assert it still succeeds and clears the other two — would pin both branches. (I verified the mechanism is sound: DeleteGrant returns sdkerrors.ErrNotFound.Wrap(...) at sei-cosmos/x/authz/keeper/keeper.go:161 and Revoke propagates it unwrapped, so errors.Is matches. It's the coverage that's missing, not the logic.)
| ) external returns (bool success); | ||
|
|
||
| /** @notice Delegate using an authorization granted by delegator. The caller supplies msg.value. */ | ||
| function delegateWithAuthorization( |
There was a problem hiding this comment.
[suggestion] "The caller supplies msg.value" describes only the precompile path and reads as a guarantee that the granter's own balance is untouched. That holds for this function but not for the underlying grant — see the note on grantStakingAuthorization. Either narrow the grant or qualify this line so the two docs don't contradict each other.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 367f8f2. Configure here.
There was a problem hiding this comment.
Well-structured addition of scoped authz flows across staking/slashing/distribution/gov, with the shared grant/exec/revoke helpers centralized and good end-to-end coverage. One blocking issue: the headline maxTokens limit is enforced by a panic inside StakeAuthorization.Accept (Coin.Sub), so an ordinary over-limit request from a grantee aborts the tx via the "EVM PANIC" path instead of reverting cleanly, and no test covers it.
Findings: 1 blocking | 13 non-blocking | 5 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- Test gap: nothing exercises exceeding
maxTokens, which is exactly the path that panics (see inline onstaking.go:623). Add cases for delegate/redelegate/undelegate amounts above the remaining allowance, and one that drains the budget to exactly zero (Acceptdeletes the grant onlimitLeft.IsZero()— worth pinning that the grant disappears). - Test gap:
RevokeAuthorizationsdocuments that "missing members are ignored so grants created before a permission was expanded can still be revoked as a group", but no test covers a partial group (only one of three staking grants present → success) or the all-missing case (→ErrNotFound). That tolerance is the whole reason the helper exists; it should be pinned. - Test gap: the new staking/slashing/distribution authorization tests don't cover the staticcall / delegatecall / payable-rejection negatives that
gov_test.goalready covers forgrantVoteAuthorization. The guards are central (Executehead), so risk is low, but the pattern is cheap to mirror. redelegateForcallsp.distributionKeeper.WithdrawDelegationRewardsdirectly (not through authz) before executing the redelegation, so a grantee holding only a redelegate grant triggers a reward withdrawal on the granter's behalf without a withdraw grant. Harmless in effect (funds route to the granter's own withdraw address) and pre-existing shared code, but worth confirming it's intended now that a third party can reach it.- The distribution refactor narrowed the
recover()scope:validateInput,getDelegator, and theargs[0].(string)/args[0].(common.Address)assertions now run inwithdrawDelegationRewards/withdrawValidatorCommissionbefore entering the*Forhelper that installs the deferred recover. ABI decoding guarantees the arg types so this is theoretical, but AGENTS.md asks that a readability refactor not change behavior — consider moving the recover to the outer entry points. Staking.soldoesn't mention that re-granting overwrites the three existing grants and therefore resets eachmaxTokensbudget. One line would prevent a caller from assuming grants accumulate.- The Cursor second-opinion pass produced no output (
cursor-review.mdis empty), so this synthesis merges only Claude's findings with Codex's. - Codex's single finding points at the right code but the wrong consequence:
maxTokensis enforced —sei-cosmos/types/coin.go:117panics rather than silently allowing a negative remainder, so there is no overspend. The actionable part of the finding (missing overspend check + tests) still stands, as detailed inline. - The
app-hash-breakinglabel plus adding methods only to the live (non-snapshot) precompile is the expected release flow here (GetVersionedmapslatestUpgrade→ current, snapshots are cut byscripts/bump_version); flagged only to note it was checked, no action needed. - 4 suggestion(s)/nit(s) flagged inline on specific lines.
|
|
||
| func (p PrecompileExecutor) authorizedStakingExecutor(ctx sdk.Context, grantee sdk.AccAddress) stakingMessageExecutor { | ||
| return func(msg sdk.Msg) error { | ||
| _, err := pcommon.ExecuteAuthorization(ctx, p.authzMsgServer, grantee, msg) |
There was a problem hiding this comment.
[blocker] The maxTokens budget is enforced by a panic, not an error. StakeAuthorization.Accept (sei-cosmos/x/staking/types/authz.go:106) computes limitLeft := a.MaxTokens.Sub(amount), and Coin.Sub panics with "negative coin amount" when the result is negative (sei-cosmos/types/coin.go:117-119).
Nothing on this path recovers it: ExecuteAuthorization → Exec → DispatchActions have no recover, delegateFor/redelegateFor/undelegateFor have no recover (unlike the distribution helpers), and DynamicGasPrecompile.RunAndCalculateGas only scopes its recover to chargeDecodeGas. So the panic reaches x/evm/keeper/msg_server.go:84, which logs EVM PANIC, calls debug.PrintStack(), increments the panics metric, and re-panics for baseapp to convert into a failed tx.
Failure scenario: granter grants maxTokens = 200; grantee calls delegateWithAuthorization with 100 (remaining budget 100), then calls it again with 150. The second call panics instead of reverting. Consequences:
- the calling contract cannot catch the failure (no
ErrExecutionReverted, the whole tx aborts with an internal error); - any grantee can trigger stack dumps and
EVM PANIClog/metric noise at will, poisoning a signal the repo clearly treats as "this is a bug".
No overspend occurs, so this isn't a fund-loss issue — but the primary safety mechanism of the feature only has a panicking error path. Suggest pre-checking the remaining allowance here (read the stored grant and compare before dispatching) so an over-limit request returns a normal error, or failing that, recovering around the exec and converting the panic into one. Please add a test for an over-limit request in either case.
| expiration := time.Unix(args[3].(int64), 0).UTC() | ||
| authorizations, err := newStakingAuthorizations( | ||
| allowedValidators, | ||
| sdk.NewCoin(sdk.MustGetBaseDenom(), sdk.NewIntFromBigInt(maxTokens)), |
There was a problem hiding this comment.
[suggestion] The grant's MaxTokens denom comes from sdk.MustGetBaseDenom(), but undelegateFor builds the message coin with p.evmKeeper.GetBaseDenom(ctx) (staking.go:578) while delegateFor/redelegateFor use sdk.MustGetBaseDenom(). If those two ever disagree, Coin.Sub inside StakeAuthorization.Accept panics on "invalid coin denominations" — the same unrecovered path as the comment above, but for every undelegation rather than only over-limit ones. Worth picking one source of truth for the denom across the grant and all three message builders.
| * @param allowedValidators Validators the grantee may target. For redelegation this limits destination validators | ||
| * @param maxTokens Maximum cumulative base-denom amount for each of delegate, redelegate, and undelegate | ||
| * @param expiration Unix timestamp after which the authorization is invalid | ||
| * @dev Creates three native StakeAuthorizations with independent maxTokens budgets. They can be used through this precompile or native Cosmos MsgExec |
There was a problem hiding this comment.
[suggestion] This @dev correctly says the grants also work through native Cosmos MsgExec, but it omits the consequence that matters most to a caller: through MsgExec the delegated tokens are debited from the granter's balance, whereas through delegateWithAuthorization the grantee supplies msg.value (line 132). So grantStakingAuthorization is effectively a spend authorization over the granter's liquid balance, bounded by maxTokens per action — which reads nothing like the payable EVM entry point right below it.
grantProposalAuthorization in Gov.sol got an explicit "grant only to a fully trusted account" warning for the analogous native-path escape hatch; the staking grant deserves the same treatment.
| * @param proposer The account on whose behalf the proposal is submitted | ||
| * @param proposalJSON JSON string containing the proposal details | ||
| * @return proposalID The ID of the created proposal | ||
| * @notice The caller supplies the initial deposit through msg.value |
There was a problem hiding this comment.
[nit] Worth adding that the deposit is credited to proposer and that any deposit refund (or burn, if the proposal is vetoed) settles against proposer, not the caller who funded it via msg.value. As written, "the caller supplies the initial deposit" reads as though the caller retains a claim on it.
| // ExecuteAuthorization routes a concrete message through the native authz | ||
| // server, preserving its message-type scope and normal grant consumption. | ||
| func ExecuteAuthorization(ctx sdk.Context, msgServer utils.AuthzMsgServer, grantee sdk.AccAddress, msg sdk.Msg) (*authztypes.MsgExecResponse, error) { | ||
| if err := msg.ValidateBasic(); err != nil { |
There was a problem hiding this comment.
[nit] ExecuteAuthorization validates the inner message here, but every caller also calls msg.ValidateBasic() immediately before invoking it (slashing.go unjail, both distribution withdraw executors, prepareSubmitProposal, and the three staking *For helpers), so each authorized message is validated twice. Per AGENTS.md ("guard at the choke point, never at each caller") pick one: keep it here and drop the authz-only call sites, or drop it here since staking's *For helpers must validate anyway to cover the direct executor. Either is fine — having both makes the invariant's owner ambiguous.
There was a problem hiding this comment.
Scoped authz flows for staking/slashing/distribution/gov are implemented consistently — shared grant/exec/revoke helpers, per-method readOnly and delegatecall guards, ABI/Solidity/IsTransaction all in sync, and the new StakeAuthorization.Accept guard correctly closes a real panic on over-limit or denom-mismatched amounts. No blocking defects found; the notes below are scoping/robustness and documentation suggestions.
Findings: 0 blocking | 9 non-blocking | 4 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Second-opinion passes: Codex reported "No material issues found in the pull request diff."
cursor-review.mdis empty — that pass produced no output, so its coverage should not be assumed. - Test coverage gap: the PR description asserts that the staking/gov constraints "apply equally through the EVM precompile and native Cosmos
MsgExec", but no test exercises the nativeMsgExecpath end-to-end (only the type-levelauthz_test.gocase). Given that the native path is where the unbounded gov deposit and granter-funded delegation become reachable, oneMsgExectest per module would pin the behavior the docs promise. - No negative test asserts that
grantWithdrawAuthorizationdoes not coverMsgSetWithdrawAddress— the gov test does exactly this scoping assertion forMsgVotevsMsgSubmitProposal, and the same shape would be cheap to add for distribution and slashing. sei-cosmos/x/staking/types/authz.gochanges consensus behavior for pre-existing nativeStakeAuthorizationgrants (over-limit requests previously panicked out ofCoin.Sub, now return a cleanErrInsufficientFunds), and it is not upgrade-gated. Theapp-hash-breakinglabel suggests this is intentional and handled by the release cut — worth an explicit confirmation from the author rather than a code change.- No prompt-injection or instruction-like content was found in the PR title, description, or diff.
- 4 suggestion(s)/nit(s) flagged inline on specific lines.
| return authz.AcceptResponse{Accept: true, Delete: false, | ||
| Updated: &StakeAuthorization{Validators: a.GetValidators(), AuthorizationType: a.GetAuthorizationType()}}, nil | ||
| } | ||
| if a.MaxTokens.Denom != amount.Denom || a.MaxTokens.Amount.LT(amount.Amount) { |
There was a problem hiding this comment.
[suggestion] Good catch closing the Coin.Sub panic here — both a denom mismatch and an over-limit amount previously panicked.
The two conditions have different meanings though, and folding them into one ErrInsufficientFunds produces a misleading error for the denom case: a MsgDelegate with, say, 1000uatom against a usei grant reports "requested amount is more than staking authorization limit", which sends the caller looking at their budget rather than their denom. Consider splitting:
if a.MaxTokens.Denom != amount.Denom {
return authz.AcceptResponse{}, sdkerrors.ErrInvalidRequest.Wrapf(
"cannot delegate %s with a %s authorization", amount.Denom, a.MaxTokens.Denom)
}
if a.MaxTokens.Amount.LT(amount.Amount) {
return authz.AcceptResponse{}, sdkerrors.ErrInsufficientFunds.Wrap(
"requested amount is more than staking authorization limit")
}| if err == nil { | ||
| dstWithdrawnCoins, wErr := p.distributionKeeper.WithdrawDelegationRewards(ctx, delegator, dstValAddr) | ||
| if wErr == nil { | ||
| dstWithdrawnCoins, withdrawErr := p.distributionKeeper.WithdrawDelegationRewards(ctx, delegator, dstValAddr) |
There was a problem hiding this comment.
[suggestion] This pre-withdraw runs against p.distributionKeeper directly, so on the redelegateWithAuthorization path the grantee triggers a WithdrawDelegationRewards for the granter that no grant authorizes. The proceeds go to the granter's own withdraw address, so nothing is stolen — but a StakeAuthorization for MsgBeginRedelegate is now sufficient to force the granter's dst-validator rewards out and reset their delegation starting info, which is outside the scope the Solidity docs describe.
Two options: route the pre-withdraw through pcommon.ExecuteAuthorization as well (which would then require a withdraw grant, changing the UX), or document on redelegateWithAuthorization in Staking.sol that redelegation authority implies a destination-validator reward withdrawal.
Separately, withdrawErr is still discarded — a failure here silently leaves dstRewardAmount at 0, and the emitted DelegationRewardsWithdrawn event then under-reports. That was pre-existing, but the rename makes it a natural moment to at least logger.Error it, matching how the emit failures below are handled.
| } | ||
|
|
||
| func (p PrecompileExecutor) grantProposalAuthorization(ctx sdk.Context, method *abi.Method, caller common.Address, args []interface{}, value *big.Int) ([]byte, uint64, error) { | ||
| return p.grantAuthorization(ctx, method, caller, args, value, &govtypes.MsgSubmitProposal{}) |
There was a problem hiding this comment.
[suggestion] This is the one grant in the PR with no bound on what it can cost the granter: GenericAuthorization over MsgSubmitProposal lets the grantee, via native MsgExec, submit with an arbitrary InitialDeposit debited from the granter — up to their whole balance, permanently lost if the proposal is vetoed or fails quorum. The precompile path caps this at the grantee's own msg.value, but the native path does not, and the grant is the same object.
The risk is documented in Gov.sol's @notice and in the PR description, so this is a deliberate call rather than an oversight. Still worth reconsidering, because it sits next to grantStakingAuthorization, which takes an explicit maxTokens — callers will reasonably read the two as equally scoped. A gov-specific Authorization implementation carrying a deposit ceiling (mirroring StakeAuthorization.MaxTokens) would make the EVM surface honest about what it hands out; failing that, naming the function something that signals the blast radius would help.
| /// @return success True if commission was withdrawn successfully | ||
| function withdrawValidatorCommission() external returns (bool success); | ||
|
|
||
| /// @notice Authorizes a grantee to withdraw delegation rewards and validator commission for the caller |
There was a problem hiding this comment.
[nit] These four new declarations carry only @notice, while every neighbouring method in this interface documents @param and @return success. Worth matching the surrounding style — in particular expiration deserves the same "Unix timestamp after which the authorization is invalid" note that Gov.sol gives it, since the unit is not inferable from int64.
Superseded: latest AI review found no blocking issues.
There was a problem hiding this comment.
Well-structured addition of scoped authz-backed permissions across the staking, gov, slashing, and distribution precompiles, with the shared grant/exec/revoke logic correctly centralized in precompiles/common/authorization.go and delegatecall/staticcall guards in place on every new mutating method. No correctness or security blockers found, but two of the security-critical negative assertions in the gov test pass for the wrong reason, the proposal authorization is unbounded in a way that is asymmetric with staking's maxTokens, and a few structural/godoc points diverge from AGENTS.md.
Findings: 0 blocking | 11 non-blocking | 6 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Cursor's second-opinion file (
cursor-review.md) is empty — that pass produced no output. Codex (codex-review.md) reported no material issues. - Godoc style: several new helpers put rationale/mechanism in their doc comments, which
AGENTS.mdsays belongs in an inline comment at the line that needs it ("Explain WHAT, not WHY or HOW", "Never record design history"). Examples:GrantGenericAuthorizations("Cosmos authz keys grants by message type, so…"),RevokeAuthorizations("…so grants created before a permission was expanded can still be revoked"),withdrawDelegationRewardsFor("…while allowing each entry point to enforce its own…"),prepareSubmitProposal("…so the two entry points cannot drift…"). StakeAuthorization.Acceptinsei-cosmos/x/staking/types/authz.gois not version-gated, unlike the precompile changes (which land as the unreleasedv6.6entry in eachsetup.go). The panic→error change therefore applies to every height a v6.6 binary executes, including natively-created grants that never touch the EVM. The PR describes this as intentional and carriesapp-hash-breaking; worth an explicit confirmation at the release cut.- Test coverage gaps for behaviors the PR description claims: grouped revocation "tolerates missing members" (every revoke test has all group members present), the all-members-missing →
ErrNotFoundpath, and grant exhaustion (Delete: truewhenmaxTokensis spent to exactly zero — the staking flow test leaves 60/170/180 remaining). expiration := time.Unix(args[N].(int64), 0)accepts anyint64. A caller-supplied expiration beyond year 9999 survivesMsgGrant.ValidateBasicand reachesSaveGrant'scdc.MustMarshal, which panics onStdTimeMarshalfailure (recovered into a revert, so no halt).GrantAuthorizationsalready validates the lower bound againstctx.BlockTime(); bounding the upper end there would make it a single choke-point check for all five grant methods.- 6 suggestion(s)/nit(s) flagged inline on specific lines.
| _, err = call( | ||
| granteeEVMAddr, | ||
| gov.SubmitWithAuthzMethod, | ||
| nil, |
There was a problem hiding this comment.
[suggestion] This negative assertion passes for the wrong reason. submitProposalWithAuthorization is payable, so value flows into prepareSubmitProposal → HandlePaymentUsei → state.SplitUseiWeiAmount(nil), which nil-derefs in big.Int.Mod. Gov's Execute recovers that panic into an error, which RunAndCalculateGas converts to vm.ErrExecutionReverted — so the call reverts before ExecuteAuthorization is ever reached, and this would pass even if a vote grant did authorize proposal submission.
Same issue at line 540 (the post-revocation call). Note line 369 already switched GrantVoteMethod to big.NewInt(0); do the same here (or pass a real deposit) so the revert actually comes from the authz check. The underlying property is still covered by the require.Nil(t, submitAuthorization) store assertions, which is why this is a test-quality point rather than a coverage hole.
| * @param grantee The account receiving the proposal authorization | ||
| * @param expiration Unix timestamp after which the authorization is invalid | ||
| * @return success Whether the authorization was successfully granted | ||
| * @notice This native MsgSubmitProposal authorization can also be used through Cosmos MsgExec with an arbitrary initial deposit debited from the caller. Grant it only to a fully trusted account; proposal deposits can be permanently lost |
There was a problem hiding this comment.
[suggestion] Worth reconsidering the asymmetry this PR introduces: grantStakingAuthorization was given an explicit maxTokens bound precisely because unbounded spend authority over the granter's liquid balance was unacceptable, but grantProposalAuthorization is a plain GenericAuthorization with no cap. Via native MsgExec a grantee can submit proposals with arbitrary initial deposits debited from the granter, repeatedly, and those deposits are burned when a proposal is vetoed or fails to reach min-deposit — so the ceiling is the granter's entire balance.
The notice here and in the PR description document this clearly, so it's a deliberate call rather than a defect. But a bounded authorization (deposit cap, analogous to maxTokens) would make the EVM-facing permission match what its name implies and would remove the native-MsgExec escalation path.
| } | ||
| authorizations := make([]authztypes.Authorization, 0, len(authorizationTypes)) | ||
| for _, authorizationType := range authorizationTypes { | ||
| authorization, err := stakingtypes.NewStakeAuthorization(allowedValidators, nil, authorizationType, &maxTokens) |
There was a problem hiding this comment.
[nit] All three StakeAuthorizations receive the same &maxTokens pointer. It's benign today — GrantAuthorizations marshals each grant independently and nothing mutates the coin between calls, and Accept builds a fresh limitLeft — but it means three logically independent budgets share one backing struct. A per-iteration copy (budget := maxTokens; ... &budget) removes the aliasing hazard for whoever touches this next.
| stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_DELEGATE, | ||
| &coin100, | ||
| stakingtypes.NewMsgDelegate(delAddr, val1, coin150), | ||
| true, |
There was a problem hiding this comment.
[nit] The table runner only does require.Error(t, err) for expectErr cases. Since the whole point of this change is that an over-limit request now returns ErrInsufficientFunds instead of panicking in Coin.Sub, this case would still pass if the error class regressed. The new denom-mismatch subtest below does this right with require.ErrorIs + ErrorContains; consider either asserting the sentinel here too or moving this case to a subtest in the same style.
| } | ||
| return p.withdrawValidatorCommission(ctx, method, caller, evm) | ||
| case GrantWithdrawMethod: | ||
| if readOnly { |
There was a problem hiding this comment.
[suggestion] The four new cases each repeat if readOnly { return … }, bringing distribution to eight copies of the same guard (staking's Execute gains four more for the same reason). AGENTS.md calls this out directly: "Guard at the choke point, never at each caller" — a guard repeated at every call site is a convention the next case can forget.
precompiles/slashing/slashing.go:107-134 is the shape to copy: query switch first, then a single EVMPrecompileCalledFromDelegateCall + readOnly pair, then the transaction switch. Distribution already computes !p.IsTransaction(method.Name) above, so the query/tx split needed to hoist the guard is already there. Non-blocking since it's extending pre-existing structure rather than introducing it.
| return p.delegateFor(ctx, method, delegator, args[0].(common.Address), args[1].(string), value, hooks, evm, p.authorizedStakingExecutor(ctx, grantee)) | ||
| } | ||
|
|
||
| // delegateFor performs the shared direct and authorized delegation steps. The |
There was a problem hiding this comment.
[nit] This godoc documents a constraint the function doesn't enforce — delegateFor never checks association; both callers resolve the delegator before calling it. Per AGENTS.md ("Document the subject, not the system", rationale belongs at the line that needs it), the association requirement and its reasoning belong on GetSeiAddressByEvmAddress/GetSeiAddressFromArg at the call sites, leaving this comment to say what delegateFor is: the shared payment, execution, and event path for direct and authorized delegation.
There was a problem hiding this comment.
Well-structured change: the new precompiles/common/authorization.go gives grant/exec/revoke a single choke point, the *For helpers keep direct and authorized paths on one event/output path, and the StakeAuthorization.Accept fix correctly replaces a Coin.Sub panic with typed denom-mismatch / insufficient-funds errors, with tests to match. No blocking correctness or security defects found; the remaining notes are design/docs/consistency suggestions.
Findings: 0 blocking | 8 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Second-opinion passes:
cursor-review.mdis empty (that pass produced no output);codex-review.mdreports no material issues. All findings below come from this pass. sei-cosmos/x/staking/types/authz.goAcceptis not upgrade-gated, so the new denom-mismatch / insufficient-funds errors change failure behavior for pre-existing nativeStakeAuthorizationgrants at the release boundary (previously aCoin.Subpanic). The PR is labeledapp-hash-breakingand calls this out, so this is only a flag for the release process, not an objection.delegateWithAuthorizationdebits the grantee'smsg.valuebut still consumes the granter'smaxTokensdelegate budget. That fails closed, but it means a grantee funding the delegation themselves burns budget intended to cap how much of the granter's own liquid balance they can move.Staking.solalready documents the msg.value asymmetry; worth adding this budget interaction to the same@devnote.Distribution.soldoesn't note (asGov.solandStaking.solnow do) thatgrantWithdrawAuthorizationcreates native Cosmos grants usable throughMsgExec, not just through this precompile. Harmless here (withdrawals only credit the granter's configured withdraw address), but the three interfaces should describe the native surface consistently.- Test coverage gaps: no case exercises
redelegateWithAuthorizationwith a destination validator outside the allow list (onlyundelegateWithAuthorizationis covered), and nothing exercises the sharedGrantAuthorizationsexpiration guard (expiration <= ctx.BlockTime()) through the new staking/slashing/distribution entry points — only the old gov path had that assertion before centralization. - 3 suggestion(s)/nit(s) flagged inline on specific lines.
| } | ||
|
|
||
| func (p PrecompileExecutor) grantProposalAuthorization(ctx sdk.Context, method *abi.Method, caller common.Address, args []interface{}, value *big.Int) ([]byte, uint64, error) { | ||
| return p.grantAuthorization(ctx, method, caller, args, value, &govtypes.MsgSubmitProposal{}) |
There was a problem hiding this comment.
[suggestion] The proposal grant is a GenericAuthorization for MsgSubmitProposal, which is unbounded on the deposit amount. Through a native MsgExec a grantee can submit proposals with an arbitrary initial deposit debited from the granter, and deposits can be burned (veto / failed quorum) — so this single permission is effectively an unbounded spend authority over the granter's liquid balance.
That's out of step with the rest of the PR: staking deliberately uses a bounded StakeAuthorization with an explicit validator allowlist and maxTokens, and the PR summary frames these as "narrowly scoped ways ... without exposing arbitrary Cosmos message execution". Consider a deposit-capped authorization for parity. If the unbounded form is the intended trade-off, the Gov.sol @notice is currently the only place that says so — it isn't visible in abi.json, so integrators generating bindings from the ABI won't see the warning at all.
| } | ||
| authorizations := make([]authztypes.Authorization, 0, len(authorizationTypes)) | ||
| for _, authorizationType := range authorizationTypes { | ||
| authorization, err := stakingtypes.NewStakeAuthorization(allowedValidators, nil, authorizationType, &maxTokens) |
There was a problem hiding this comment.
[nit] &maxTokens is the same pointer for all three authorizations, and NewStakeAuthorization stores it directly (a.MaxTokens = amount). It's safe today — the grants are marshaled to proto immediately and Accept builds a fresh limitLeft rather than mutating — but three authorizations sharing one *sdk.Coin is an aliasing hazard for anything added later. Copying inside the loop (budget := maxTokens; ... &budget) removes the coupling for free.
| // ExecuteAuthorization routes a concrete message through the native authz | ||
| // server, preserving its message-type scope and normal grant consumption. | ||
| func ExecuteAuthorization(ctx sdk.Context, msgServer utils.AuthzMsgServer, grantee sdk.AccAddress, msg sdk.Msg) (*authztypes.MsgExecResponse, error) { | ||
| if err := msg.ValidateBasic(); err != nil { |
There was a problem hiding this comment.
[nit] ExecuteAuthorization validates the message here, which is the right choke point — but every authorized caller also calls msg.ValidateBasic() immediately before invoking it (gov.voteWithAuthorization, slashing.unjailWithAuthorization, both distribution execute closures). Those caller-side guards are now redundant and are exactly the "guard repeated at every call site" pattern AGENTS.md warns against.
Note the staking ones are not redundant: delegateFor/redelegateFor/undelegateFor validate before execute(msg) because the direct executor bypasses this function. (That is a small behavior change for the direct paths — previously the keeper produced the error — but it only alters the error text, not whether the call reverts.)
| if len(authorizations) == 0 { | ||
| return sdkerrors.ErrInvalidRequest.Wrap("at least one authorization is required") | ||
| } | ||
| if !expiration.After(ctx.BlockTime()) { |
There was a problem hiding this comment.
We are only checking the lower bound.
Given expiration is a user provided value we should also check the upper bound. This one is a blocker given how the grant is marshalled.
2 participants
Summary
Why
EVM callers need narrowly scoped ways to let another associated account perform common Cosmos module actions without exposing arbitrary Cosmos message execution. Each EVM-facing permission maps to the native authorization enforced by Cosmos authz.
Staking permission
grantStakingAuthorizationrequires an explicit allowed-validator list andmaxTokensin the base denomination. It creates three nativeStakeAuthorizationgrants forMsgDelegate,MsgBeginRedelegate, andMsgUndelegate.Each action has an independent cumulative
maxTokensbudget. The validator allowlist applies to delegation and undelegation targets and to redelegation destinations. These constraints apply equally through the EVM precompile and native CosmosMsgExec. Native delegation spends the granter's liquid balance, while the payable EVM delegation method uses the grantee'smsg.value.Requests above the remaining budget return a normal insufficient-funds authorization error without consuming the grant or panicking. Denomination mismatches return a distinct invalid-request error. This changes failure behavior for pre-existing native
StakeAuthorizationgrants and is an intentional consensus-affecting change covered by the PR'sapp-hash-breakingrelease process.Authorized redelegation preserves the existing precompile behavior of pre-withdrawing accrued destination-validator rewards to the delegator's configured withdrawal address before redelegating. The Solidity interface now documents that side effect, and unexpected pre-withdraw failures are logged.
Governance permissions
Governance authorization is split by capability:
grantVoteAuthorization/revokeVoteAuthorizationmanage onlyMsgVote; they do not authorize proposal submission or weighted votesgrantProposalAuthorization/revokeProposalAuthorizationmanage onlyMsgSubmitProposalThe proposal grant is intentionally a native
GenericAuthorization. A grantee can use it through the EVM precompile or through a native CosmosMsgExec. The native path can specify an arbitrary initial deposit debited from the granter, and governance deposits can be permanently lost. The Solidity interface therefore warns callers to grant proposal authority only to fully trusted accounts.Behavior
MsgDelegate,MsgBeginRedelegate, andMsgUndelegategrantsMsgVoteMsgSubmitProposalMsgUnjailMsgWithdrawDelegatorRewardandMsgWithdrawValidatorCommission, notMsgSetWithdrawAddressmsg.valuewhile executing the native message for the granterMsgExecintegration tests pin granter-funded staking delegation and proposal submissionValidation
go test ./precompiles/...go test ./sei-cosmos/x/staking/...go test -race ./sei-cosmos/x/staking/types ./precompiles/staking ./precompiles/gov ./precompiles/distribution ./precompiles/slashinggo vet ./sei-cosmos/x/staking/types ./precompiles/staking ./precompiles/gov ./precompiles/distribution ./precompiles/slashinggo test ./appgofmtandgoimportsgit diff --check