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

feat: sign eip-712 message instead of hash #57

Merged
merged 3 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
banteg marked this conversation as resolved.
Show resolved Hide resolved
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 StaticFeeTransaction, 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 @@ -677,7 +674,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)
banteg marked this conversation as resolved.
Show resolved Hide resolved
new_signatures = get_signatures(safe_tx, signers)
sigs_by_signer = {**sigs_by_signer, **new_signatures}

if (
Expand Down Expand Up @@ -744,7 +741,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 @@ -762,20 +759,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,
)
Loading