Skip to content

Commit

Permalink
feat: sign eip-712 message instead of hash (#57)
Browse files Browse the repository at this point in the history
* feat: sign eip-712 message instead of hash

see also https://docs.safe.global/advanced/smart-account-signatures

* lint: import

* fix: cli signing
  • Loading branch information
banteg authored Jun 11, 2024
1 parent 4889ed7 commit 70234dc
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 26 deletions.
2 changes: 1 addition & 1 deletion ape_safe/_cli/pending.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def propose(cli_ctx, ecosystem, safe, data, gas_price, value, receiver, nonce, s
)
safe_tx = safe.create_safe_tx(txn)
safe_tx_hash = get_safe_tx_hash(safe_tx)
signatures = get_signatures(safe_tx_hash, safe.local_signers)
signatures = get_signatures(safe_tx, safe.local_signers)

num_confirmations = 0
submitter = sender if isinstance(sender, AccountAPI) else None
Expand Down
30 changes: 5 additions & 25 deletions ape_safe/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from ape.utils import ZERO_ADDRESS, cached_property
from ape_ethereum.transactions import TransactionType
from eip712.common import create_safe_tx_def
from eth_account.messages import encode_defunct
from eth_utils import keccak, to_bytes, to_int
from ethpm_types import ContractType

Expand Down Expand Up @@ -179,16 +178,14 @@ def _get_path(self, alias: str) -> Path:


def get_signatures(
safe_tx_hash: str,
safe_tx: SafeTx,
signers: Iterable[AccountAPI],
) -> Dict[AddressType, MessageSignature]:
signatures: Dict[AddressType, MessageSignature] = {}
for signer in signers:
message = encode_defunct(hexstr=safe_tx_hash)
signature = signer.sign_message(message)
signature = signer.sign_message(safe_tx)
if signature:
signature_adjusted = adjust_v_in_signature(signature)
signatures[signer.address] = signature_adjusted
signatures[signer.address] = signature

return signatures

Expand Down Expand Up @@ -669,7 +666,7 @@ def skip_signer(signer: AccountAPI):
# NOTE: It is okay to have less signatures, but it never should fetch more than needed
signers = [x for x in available_signers if x.address not in sigs_by_signer]
if signers:
new_signatures = get_signatures(safe_tx_hash, signers)
new_signatures = get_signatures(safe_tx, signers)
sigs_by_signer = {**sigs_by_signer, **new_signatures}

if (
Expand Down Expand Up @@ -736,7 +733,7 @@ def add_signatures(
][:amount_needed]

safe_tx_hash = _get_safe_tx_id(safe_tx, confirmations)
signatures = get_signatures(safe_tx_hash, signers)
signatures = get_signatures(safe_tx, signers)
if signatures:
self.client.post_signatures(safe_tx_hash, signatures)

Expand All @@ -754,20 +751,3 @@ def _get_safe_tx_id(safe_tx: SafeTx, confirmations: list[SafeTxConfirmation]) ->
return value

raise ApeSafeError("Failed to get transaction hash.")


def adjust_v_in_signature(signature: MessageSignature) -> MessageSignature:
MIN_VALID_V_VALUE_FOR_SAFE_ECDSA = 27
v = signature.v

if v < MIN_VALID_V_VALUE_FOR_SAFE_ECDSA:
v += MIN_VALID_V_VALUE_FOR_SAFE_ECDSA

# Add 4 because we signed with the prefix.
v += 4

return MessageSignature(
v=v,
r=signature.r,
s=signature.s,
)

0 comments on commit 70234dc

Please sign in to comment.