search

llamaxyz / llama

Llama is an onchain governance and access control framework for smart contracts.
https://llama.xyz
MIT License
47 stars 5 forks source link

fix: `LlamaAccount` execute low-level call should use value from a parameter instead of `msg.value` #460

Closed 0xrajath closed 1 year ago

0xrajath commented 1 year ago

From Spearbit Discord:

We chatted about it last time but wanted to run it by you again. The LlamaAccount.execute allows tx.origin to include some eth in their call, which will then be included in the call from LlamaAccount.execute

What are some intended use cases of eth being involved in a tx but coming from the caller and not the account itself?

The account cannot for example, use call to deposit eth into weth, trade eth on uniswap, etc.

The account can always use a delegate call out to code with the ability to include the account's own eth in a call but it seems like a lot of steps for a common use case.

Can you remind us the logic around only allowing calls from the account.execute to only use eth that is essentially donated by the original caller?
0xrajath commented 1 year ago

image

AustinGreen commented 1 year ago

We should also add a check that msg.value == 0 and revert otherwise

0xrajath commented 1 year ago

We should also add a check that msg.value == 0 and revert otherwise

Why do we need this? The call from executor to this function will outright revert anyway right?