fix: allow owners who are also delegates to sign with multiple signatures #2726
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a bug where an address that is both a Safe owner AND registered as a delegate/proposer cannot participate in multi-signature transactions.
When a user is both an owner and a proposer for a Safe, any transaction they create becomes "stuck" - other owners cannot add their confirmations because the transaction service incorrectly applies the delegate signature restriction.
Workaround
Users can remove the owner from the Proposers list. However, this should still be fixed - the UI allows this configuration without warning, the error message is confusing, and being a delegate shouldn't reduce an owner's capabilities.
Root Cause Analysis
The Bug
In
SafeMultisigTransactionSerializer.validate(), the delegate check doesn't account for addresses that are both owners AND delegates:This triggers whenever ANY signer in the transaction is in the delegates list, even if they're also an owner.
How the Safe Wallet Triggers This
Looking at
safe-wallet-monorepo/apps/web/src/services/tx/tx-sender/dispatch.ts, thedispatchTxProposalfunction handles both new transactions AND confirmations on existing transactions:When a user clicks "Confirm" on an existing transaction, the wallet:
/v1/safes/{address}/multisig-transactions/) with ALL accumulated signaturesThis means confirmations go through
SafeMultisigTransactionSerializer, notSafeMultisigConfirmationSerializer.The Failure Scenario
0x0247...) is both an owner (1 of 6) and a proposer for the Safe0xb85f...), a different owner, clicks "Confirm"delegateslen(parsed_signatures) > 1(now 2), throws: "Just one signature is expected if using delegates"The Fix
The additional
owner not in safe_ownerscheck ensures:Test Plan
test_post_multisig_transactions_with_owner_who_is_also_delegatefor v1 APItest_post_multisig_transactions_with_owner_who_is_also_delegatefor v2 API🤖 Generated with Claude Code
Note
Adjust delegate restriction to exclude Safe owners from the single-signature rule and add tests for v1/v2 APIs.
SafeMultisigTransactionSerializer.validateinsafe_transaction_service/history/serializers.pyto only enforce the delegate single-signature rule when signer is indelegatesand not insafe_owners.test_post_multisig_transactions_with_owner_who_is_also_delegatetohistory/tests/test_views.pyandhistory/tests/test_views_v2.pyto verify owner-delegates can submit multiple signatures successfully.Written by Cursor Bugbot for commit 7068044. This will update automatically on new commits. Configure here.