Closed snowtigersoft closed 5 months ago
Great idea. Instead of creating a new function, perhaps we can pass the payer (self
or self.signer
) in as address argument, or it could even just be a boolean flag.
Great idea. Instead of creating a new function, perhaps we can pass the payer (
self
orself.signer
) in as address argument, or it could even just be a boolean flag.
I think either calling transfer_by_signer_public
or by adding a signer parameter is dangerous because the contract can transfer the user's assets without the user's knowledge or care.
This means that every time a user interacts with a program, he has to look carefully at the program code to make sure it doesn't steal his public aleo credits.
However, this feature is necessary to make aleo credits and other programs to be composable.
So, it might be better and safer to follow the erc20/arc20, i.e. to add the approve
and transferFrom
functions to the credits.aleo
program.
There's danger in either path but approvals is my preferred option. It should be somewhat safer. It would be very nice to include this in the default credits.aleo program as I see forcing the wrapping of tokens to be an anti-pattern that will only make it more difficult for DApps to support many tokens.
The approve
method also has potential issues. If a contract requests an unrestricted or very high amount of authorization, it can transfer the user's assets without their knowledge. This has happened many times in the Ethereum ecosystem. As for the use of transfer_by_signer_public
, in essence, it's no different from transfer_private
. Currently, a contract can transfer a user's private tokens using transfer_private
, and users need to carefully review this to notice it. This is not fundamentally different from supporting transfer_by_signer_public
.
Yes, there's danger in either path. In any case, if you call a sign a dangerous contract either design will permit draining all of your funds.
It's really a question of how pooling assets should work. Approvals enable users to control their own funds without depositing them in a pool. This enables a user to interact with many DApps simultaneously with the same pool of funds where a transfer is effectively changing who controls the funds so the user can't use the same pool of funds in parallel with another DApp.
For example, we could create a program that acts as a trusted oracle about which DApps have been hacked. Users could give it an u64 max approval and it could monitor other approvals. If something gets hacked, it can automatically transfer away your funds to a safe place.
I think approvals should be the preferred solution but nothing is perfectly safe but I do believe approvals offer some interesting new types of user experiences.
Ultimately, I think we should head in the direction of Permit2: https://blog.uniswap.org/permit2-and-universal-router so we can get gasless approvals but just signing messages but that may be a little too ambitious right now.
Yes, the approvals method is quite important and necessary. Regarding transfer_by_signer_public
, what we actually need is something akin to Ethereum's method of carrying value
during a call, so the wallet can also alert the user about the impending expenditure of funds.
@snowtigersoft it doesn't seem that transfer_public_to_private_as_signer
is necessary once you have transfer_public_as_signer
since you can accomplish the functionality with two calls. Do you agree?
@snowtigersoft it doesn't seem that
transfer_public_to_private_as_signer
is necessary once you havetransfer_public_as_signer
since you can accomplish the functionality with two calls. Do you agree?
I don't think it works like that. When you invoke transfer_public_as_signer
, the ACs are transferred to the receiver's account. How does the program then convert these ACs into private? This can only be done by the receiver. It's not feasible within the same program call.
I understand what you're saying. transfer_public_to_private_as_signer
can achieve the same result as transfer_public_as_signer
+ transfer_public_to_private
, but the signers for these two methods are different. Therefore, it's not possible to execute them simultaneously in a single call unless the signer and receiver are the same account.
Here's a program that demonstrates the desired functionality
import credits.aleo;
program credits_wrapper.aleo;
function transfer_public_to_private:
input r0 as address.private;
input r1 as u64.public;
call credits.aleo/transfer_public_as_signer credits_wrapper.aleo r1 into r2;
call credits.aleo/transfer_public_to_private r0 r1 into r3 r4;
async transfer_public_to_private r2 r4 into r5;
output r3 as credits.aleo/credits.record;
output r5 as credits_wrapper.aleo/transfer_public_to_private.future;
finalize transfer_public_to_private:
input r0 as credits.aleo/transfer_public_as_signer.future;
input r1 as credits.aleo/transfer_public_to_private.future;
contains credits.aleo/account[credits_wrapper.aleo] into r2;
assert.eq r2 false;
await r0;
get credits.aleo/account[credits_wrapper.aleo] into r3;
assert.eq r3 r0[2u32];
await r1;
An associated test can be found here
🚀 Feature
I propose adding two new methods to
credits.aleo
, tentatively namedtransfer_by_signer_public
andtransfer_by_signer_public_to_private
. These new methods would be similar to the existingtransfer_public
andtransfer_public_to_private
, but the payer in these new methods should be changed fromself.caller
toself.signer
.Motivation
Currently, when
transfer_public
andtransfer_public_to_private
are invoked via a program, the cost is deducted from the program's account, not the actual initiator's (i.e., the signer's) account. This may not be the desired behavior in some scenarios.For example, if a smart contract needs to make transfers on behalf of a user, the existing design deducts credits from the smart contract itself, rather than from the user (the signer). This could not only potentially deplete the smart contract's credits unexpectedly but may also introduce security and permission-related concerns.
By introducing these two new methods, we can ensure that the cost is correctly deducted from the signer's account, which is more logical and safer.
Implementation
transfer_by_signer_public
andtransfer_by_signer_public_to_private
, in thecredits.aleo
file.transfer_public
andtransfer_public_to_private
, butself.caller
should be replaced withself.signer
.Affected components could include:
credits.aleo
fileAre you willing to open a pull request?
Yes, I am willing to submit a pull request and engage in subsequent code reviews and testing.