Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add method to remove delegate with provided signature #981

Merged
merged 8 commits into from
Jun 5, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,37 @@ def remove_delegate(
raise SafeAPIException(f"Cannot remove delegate: {response.content}")
return True

def remove_delegate_signed(
Copy link
Member

@moisses89 moisses89 May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like to have two functions to remove delegates. We have here two options in my opinion.

  • Remove the function that is signing inside
  • or remove_delegate with optional signature and optional signature parameters as the signer.

I prefer the first one, because in my opinion calculate the signature is out of the goal of a client API function.
For both options we should set the label breaking_change for the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, makes sense. Removed here a7eb692

self,
delegator_address: ChecksumAddress,
delegate_address: ChecksumAddress,
signature: bytes,
safe_address: Optional[ChecksumAddress] = None,
) -> bool:
"""
Deletes a delegated user

:param delegator_address:
:param delegate_address:
:param signature: Signature of a hash of an eip712 message.
:param safe_address: If specified, a delegate is removed for a delegator for the specific safe.
Otherwise, the delegate is deleted in a global form.
:return:
"""
remove_payload = {
"delegator": delegator_address,
"signature": HexBytes(signature).hex(),
}
if safe_address:
remove_payload["safe"] = safe_address
response = self._delete_request(
f"/api/v2/delegates/{delegate_address}/",
remove_payload,
)
if not response.ok:
raise SafeAPIException(f"Cannot remove delegate: {response.content}")
return True

def post_transaction(self, safe_tx: SafeTx) -> bool:
random_sender = "0x0000000000000000000000000000000000000002"
sender = safe_tx.sorted_signers[0] if safe_tx.sorted_signers else random_sender
Expand Down
58 changes: 58 additions & 0 deletions gnosis/safe/tests/api/test_transaction_service_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from django.test import TestCase

from eth_account import Account
from hexbytes import HexBytes

from gnosis.eth import EthereumClient, EthereumNetwork, EthereumNetworkNotSupported
Expand Down Expand Up @@ -183,3 +184,60 @@ def test_get_safe_transaction(self):
f"Cannot get transaction with safe-tx-hash={safe_tx_hash.hex()}:",
str(context.exception),
)

def test_remove_delegate(self):
with patch.object(
TransactionServiceApi, "_delete_request"
) as mock_delete_request:
delegate_address = Account().create().address
delegator_account = Account().create()
self.transaction_service_api.remove_delegate(
self.safe_address, delegate_address, delegator_account
)
expected_hash = self.transaction_service_api.create_delegate_message_hash(
delegate_address
)
expected_sign = delegator_account.signHash(expected_hash)
expected_url = f"/api/v2/delegates/{delegate_address}/"
expected_payload = {
"safe": self.safe_address,
"delegator": delegator_account.address,
"signature": expected_sign.signature.hex(),
}
mock_delete_request.assert_called_once_with(expected_url, expected_payload)

def test_remove_delegate_signed(self):
with patch.object(
TransactionServiceApi, "_delete_request"
) as mock_delete_request:
delegate_address = Account().create().address
delegator_account = Account().create()
message_hash = self.transaction_service_api.create_delegate_message_hash(
delegate_address
)
signature = delegator_account.signHash(message_hash).signature.hex()
self.transaction_service_api.remove_delegate_signed(
delegator_account.address, delegate_address, signature
)

expected_url = f"/api/v2/delegates/{delegate_address}/"
expected_payload = {
"delegator": delegator_account.address,
"signature": signature,
}
mock_delete_request.assert_called_once_with(expected_url, expected_payload)

self.transaction_service_api.remove_delegate_signed(
delegator_account.address,
delegate_address,
signature,
self.safe_address,
)

expected_url = f"/api/v2/delegates/{delegate_address}/"
expected_payload = {
"safe": self.safe_address,
"delegator": delegator_account.address,
"signature": signature,
}
mock_delete_request.assert_called_with(expected_url, expected_payload)
Loading