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

Op code to receive the app call sender #6065

Closed
scholtz opened this issue Jul 12, 2024 · 24 comments
Closed

Op code to receive the app call sender #6065

scholtz opened this issue Jul 12, 2024 · 24 comments
Labels
new-feature-request Feature request that needs triage

Comments

@scholtz
Copy link
Contributor

scholtz commented Jul 12, 2024

Problem

We want to check the legitimity of address who calls the application. We can do outer information check up to level 1 at the moment (to receive the txn.Sender) . However we want our smart contracts to be composible, so that people can call it within the inner tx groups. There is no way to receive the information about the real signer in the smart contract now. Please enable it.

Our business use case:

We have created Biatec concentrated liquidity AMM and we need to check the KYC verification level of real sender, and we want to incentivize users to use our product so that they receive higher engagement level and they will have lower swapping fees. Inability to check the real sender breaks the composibility in the dex aggregators.

https://github.com/scholtz/BiatecCLAMM/blob/9906610e25108cb79e63e8e788c64177f016819f/contracts/BiatecClammPool.algo.ts#L888

Solution

Please create opcode to receive the real app call sender.
Please allow usage of it in the python/tealscript

for example as txn.globalSender or globals.sender

Dependencies

no

Urgency

We want to launch the Biatec concentrated liquidity with this feature, so it is super high urgent.

@scholtz scholtz added the new-feature-request Feature request that needs triage label Jul 12, 2024
@emg110
Copy link
Contributor

emg110 commented Jul 12, 2024

You can do it now (without the need for any change in Algorand source code or new opcodes) :
In smart contract which originates the inner TXN, add txn sender to accounts array or ARGs of your inner transaction and within the destination smart contract read it while handling method call. You can use this in all subsequent nested inner txn calls as well and keep real transaction sender record during call chain.

@scholtz
Copy link
Contributor Author

scholtz commented Jul 12, 2024

You can do it now (without the need for any change in Algorand source code or new opcodes) : In smart contract which originates the inner TXN, add txn sender to accounts array or ARGs of your inner transaction and within the destination smart contract read it while handling method call. You can use this in all subsequent nested inner txn calls as well and keep real transaction sender record during call chain.

But if i have it as the argument i cannot verify if the sender is really the address specified. This way anybody could send there some best trader address, and they would receive the lowest trading fees..

I have no control over the smart contracts which call my contract.. as i said we need this composable.. the same way as dex aggregators can call pact and tinyman, but we want to have the information of the sender because of the verification level check and the level of fees user has to pay so that we motivate engagement to use our services

@emg110
Copy link
Contributor

emg110 commented Jul 12, 2024

The smart contract conventions are author's to determine, if you set it at an account array index and receive it from that in destination contract, no one can change it or take advantage of it because it is an end to end internal op for SC domain.

@scholtz
Copy link
Contributor Author

scholtz commented Jul 12, 2024

I have no control over the smart contracts which call my contract.. as i said we need this composable.. the same way as dex aggregators can call pact and tinyman, but we want to have the information of the sender because of the verification level check and the level of fees user has to pay so that we motivate engagement to use our services

I have no control over the smart contracts which call my contract.. as i said we need this composable.. the same way as dex aggregators can call pact and tinyman, but we want to have the information of the sender because of the verification level check and the level of fees user has to pay so that we motivate engagement to use our services

@emg110
Copy link
Contributor

emg110 commented Jul 12, 2024

Oh having no control is another context you just updated and added to this subject. yes in that case this solution I mentioned is of no use.

@scholtz
Copy link
Contributor Author

scholtz commented Jul 13, 2024

Oh having no control is another context you just updated and added to this subject. yes in that case this solution I mentioned is of no use.

So I assume you agree to create opcode to allow smart contracts be able to use the real sender? I believe it opens a lot of use cases which are not possible at the moment.

@emg110
Copy link
Contributor

emg110 commented Jul 14, 2024

Yes, I agree it can be very useful to get not only the original sender but also all of the original TXN information during ITXN handling in destination SC.

@pbennett
Copy link

Perhaps always providing access to the 'outer transaction group' and for a contract, it can know the current outer index its within.

I think there could be some value here but how does this not make the contract unable to be composable (or is that ultimately, the intent?) Contract Z being called by user A, or contract A - normally - really shouldn't care. Same with contract A calling B, which calls C, which calls.. Z. Look at AMM swaps for eg, swap aggregators slice and dice those quite a bit and the swap contracts are perfectly fine with it.
If you want to make yourself only usable from approved outer level you can just force transaction group ordering and have Gtxn[0] for eg be by an approved sender (which you're saying you need for your usecase). You could ensure no contract caller would meet that criteria.

@jannotti
Copy link
Contributor

(or is that ultimately, the intent?)

Yes, I think that is the goal, viewed from a security perspective. Ethereum has access to tx.origin and msg.sender.

https://medium.com/@solidity101/demystifying-ethereum-understanding-msg-sender-tx-origin-and-external-calls-dbf02d9e947d

tx.origin is approximately what's being asked for here, as it is the original top-level initiator of an Ethereum contract call.

Most things I have read say it is bad practice to use that top-level caller for authorization. I believe the concern is that it allows a malicious contract to invoke a contract and essentially inherit the rights of its caller. That gives me enough pause to consider whether it's a good idea to allow this. If security of this kind is important, I'm not sure this kind of contract should be arbitrarily composable. Perhaps it should only allow the callers it wants including only allowing approved intermediate contracts. It seems odd to demand the original caller is A, but not mind that an intermediate, unknown contract is operating on A's behalf.

@scholtz
Copy link
Contributor Author

scholtz commented Jan 10, 2025

@pbennett

The use case is - Biatec Identity verifies person and stores to the box verification class 2 or trading fees multiplier, Biatec dex allows to swap only with verification class 2 verified accounts. so in general there is contract I (identity) and contract E (exchange or amm) .. now there will come folks router who will want to do dex aggregation and call multiple smart contracts.. They create smart contract F.. now the user calls contract F, which calls contract E which checks the box at contract I. This use case is now not possible because the E can just check the one level up sender which is the F contract address, and can fetch the box from I from wrong sender. Thus because the user in biated identity box has trading fees set to 50% level because he is super good trader, because he is using the folks router he cannot execute it, nor the folks address is verified so the swap fails.

Now this is possible to do only with offchain dex aggregators (because only one level of caller ownership is proovable).. if this would be enabled the onchain dex aggregators can come in place.

@jannotti
Copy link
Contributor

jannotti commented Jan 10, 2025

@pbennett

The use case is - Biatec Identity verifies person and stores to the box verification class 2 or trading fees multiplier, Biatec dex allows to swap only with verification class 2 verified accounts. so in general there is contract I (identity) and contract E (exchange or amm) .. now there will come folks router who will want to do dex aggregation and call multiple smart contracts.. They create smart contract F.. now the user calls contract F, which calls contract E which checks the box at contract I. This use case is now not possible because the E can just check the one level up sender which is the F contract address, and can fetch the box from I from wrong sender. Thus because the user in biated identity box has trading fees set to 50% level because he is super good trader, because he is using the folks router he cannot execute it, nor the folks address is verified so the swap fails.

Let's look at this under the perspective I described above. If we allowed this behavior, Folks could make their contract do this: "Oh, nice, since A is calling us, we can make cheap trades. Let's do some other trades, on behalf of other accounts, at the discount rate".

Now, you probably think "Folks is trustworthy, they won't do that". If you believe that, great, you should allow the Folks contract to call your contract and they would include an argument to specify who they are calling it on behalf of. You trust them not to lie.

Your contract gets a new argument to specify who it is being called on behalf of. You trust it if the sender matches that argument, or the sender is a contract that you have on your allow-list.

@scholtz
Copy link
Contributor Author

scholtz commented Jan 10, 2025

@pbennett
The use case is - Biatec Identity verifies person and stores to the box verification class 2 or trading fees multiplier, Biatec dex allows to swap only with verification class 2 verified accounts. so in general there is contract I (identity) and contract E (exchange or amm) .. now there will come folks router who will want to do dex aggregation and call multiple smart contracts.. They create smart contract F.. now the user calls contract F, which calls contract E which checks the box at contract I. This use case is now not possible because the E can just check the one level up sender which is the F contract address, and can fetch the box from I from wrong sender. Thus because the user in biated identity box has trading fees set to 50% level because he is super good trader, because he is using the folks router he cannot execute it, nor the folks address is verified so the swap fails.

Let's look at this under the perspective I described above. If we allowed this behavior, Folks could make their contract do this: "Oh, nice, since A is calling us, we can make cheap trades. Let's do some other trades, on behalf of other accounts, at the discount rate".

Now, you probably think "Folks is trustworthy, they won't do that". If you believe that, great, you should allow the Folks contract to call your contract and specify who they are calling it on behalf of. You trust them not to lie.

Yes i agree.. if folks would have malicious smart contract which would be traded on the user account, then they can do it.. however i say it is user right to sign the app call to the smart contracts he trusts

From my perspective as identity verifier and dex i want people like folks to be able to compose my smart contracts to get the better trading results.. and for each real signer i want with his engagement level to receive a bonus he deserves.. and if there be malicious trading activity made for third party for example in institutional level verification class, then this would be marketized and those institutions will not sign any more the app calls from that smart contract provider.. and btw you cant just use funds of other accounts with third party signature, it would be quite crucial security bug for some protocol..

@jannotti
Copy link
Contributor

jannotti commented Jan 10, 2025

@pbennett
The use case is - Biatec Identity verifies person and stores to the box verification class 2 or trading fees multiplier, Biatec dex allows to swap only with verification class 2 verified accounts. so in general there is contract I (identity) and contract E (exchange or amm) .. now there will come folks router who will want to do dex aggregation and call multiple smart contracts.. They create smart contract F.. now the user calls contract F, which calls contract E which checks the box at contract I. This use case is now not possible because the E can just check the one level up sender which is the F contract address, and can fetch the box from I from wrong sender. Thus because the user in biated identity box has trading fees set to 50% level because he is super good trader, because he is using the folks router he cannot execute it, nor the folks address is verified so the swap fails.

Let's look at this under the perspective I described above. If we allowed this behavior, Folks could make their contract do this: "Oh, nice, since A is calling us, we can make cheap trades. Let's do some other trades, on behalf of other accounts, at the discount rate".
Now, you probably think "Folks is trustworthy, they won't do that". If you believe that, great, you should allow the Folks contract to call your contract and specify who they are calling it on behalf of. You trust them not to lie.

Yes i agree.. if folks would have malicious smart contract which would be traded on the user account, then they can do it.. however i say it is user right to sign the app call to the smart contracts he trusts

From my perspective as identity verifier and dex i want people like folks to be able to compose my smart contracts to get the better trading results.. and for each real signer i want with his engagement level to receive a bonus he deserves.. and if there be malicious trading activity made for third party for example in institutional level verification class, then this would be marketized and those institutions will not sign any more the app calls from that smart contract provider.. and btw you cant just use funds of other accounts with third party signature, it would be quite crucial security bug for some protocol..

The caller is not victimized in the attack I described. You (Biatec) are. People are getting the cheap trading fees that you intended only for user A. So, you might imagine Folks (or a less savory aggregator) using this technique, then giving kickbacks to A for enabling them to trade cheaply with A's credentials. A is happy to keep signing. You should not want the ability to make this mistake.

@scholtz
Copy link
Contributor Author

scholtz commented Jan 10, 2025

People are getting the cheap trading fees that you intended only for user A.

can you please describe more in depth how you mean this?

i believe if person A which does pays 50% of standard fees at biatec dex sign the transaction, he cannot use funds of the person B because it would require grouped transaction where also person B releases the funds for trading.. If this is the case you are trying to reference, the person A is institution and is fully aware that on their identity is trading someone else.. And this is not a problem for me because if goverment agency will ask for info, we can point the finger to person A who is responsible

if you are trying to say that folks will verify their identity and make all people trade through them, we let it be.. if they want to do custodian services we do not have problem with it.. until the day comes and someone will report that stolen funds are there and we block that account from trading

@jannotti
Copy link
Contributor

can you please describe more in depth how you mean this?

When a DEX calls your contract, it is doing so with its own funds. A transfers to the DEX, then the DEX uses the funds to perform the trade with you, then it returns the results to A. That normally happens in one transaction group. Biatec doesn't really know the money was A's. You are hoping that this new facility would help you to know the money was A's, so you can give the DEX better fee structure when calling on behalf of A.

I propose that the DEX can take advantage of your generosity. It advertises "If you deposit funds and trade instructions with me, whenever I get a chance, I'll trade those funds using someone else's credentials, and give you a better deal."

Three group transactions are required, they may be in separate blocks:

  1. B deposits some money at the DEX along with instructions to perform a swap, and waits. (Presumably it could withdraw these funds if it changes its mind before group 2 happens.)

  2. A asks the DEX to perform an exchange on its behalf. The DEX, seeing that it has a pending request to perform a swap for B, does both when called by A. It doesn't steal A's funds, but it perform two trades, claiming to be on behalf of A each time. These funds are done with money that is currently in the DEX's account, there's no use of B's funds by A or vice versa. A gets back what it should (and maybe a kickback?)

  3. B withdraws the asset from the DEX that it acquired for it during group 2.

The only solution here is for you, Biatec, to decide which DEXs can call you, because you trust them, not to implicitly assume that if the original caller is A, the trade must be from A.

@scholtz
Copy link
Contributor Author

scholtz commented Jan 10, 2025

The only solution here is for you, Biatec, to decide which DEXs can call you, because you trust them, not to implicitly assume that if the original caller is A, the trade must be from A.

I agree. This custodian solution may be created by someone. I dont have problem with it.. This way we verify A identity and they may trigger the DEX smart contract where they perform also B swap. The person A is fully aware that they are doing this and it is their right to use it or not use it.

However the point is that to be able to have this usecase we need to know the global app call sender and i still dont see the problem for us.. The problem for me is that i dont know the real sender identity now and i want the institutions to be traded between each other and have ability to block all scammers on the network, and also we need efficient dex aggregators.

For this i need just tx.originSender field in the tx structure.

@scholtz
Copy link
Contributor Author

scholtz commented Jan 10, 2025

https://medium.com/@solidity101/demystifying-ethereum-understanding-msg-sender-tx-origin-and-external-calls-dbf02d9e947d

Btw if you implement this you remove one argument why ethereum is better then algorand.. With ETH you can work with the signer of the app call in the smart contract and in the algorand you can not at the moment. (Off course the algorand is much bether then eth but this may be an argument from the eth maxi)

@jannotti
Copy link
Contributor

jannotti commented Jan 10, 2025

The person A is fully aware that they are doing this and it is their right to use it or not use it.

This is not about A. They are indeed willing to sign such an app call, and they are not hurt by it. Instead, you the app using the facility you are asking for, are harmed.

However the point is that to be able to have this usecase we need to know the global app call sender and i still dont see the problem for us.

I don't understand how you can read all of that, agree with it, and then say that you want to do it anyway. It is wrong for a contract to base security decisions on the original sender, when an intermediate app is involved. Your example was perfect. You the contract were trying to enforce a rule, but you have been tricked. You should not blindly trust the original sender (if we made it available), you must decide exactly which users and/or apps to trust.

There are any number of Ethereum vulnerabilities from people who write contracts like you've described. Those contracts are the ones harmed, not the callers. I don't want people to write those contracts and be harmed.

https://medium.com/coinmonks/smart-contract-security-tx-origin-authorization-attack-vectors-027730ae601d

This is an argument why Algorand is better, not vice versa. It is clear that Ethereum designers consider it a mistake. From that article:

Besides the issue with authorization, there is a chance that tx.origin will be removed from the Ethereum protocol in the future, so code that uses tx.origin won’t be compatible with future releases Vitalik: ‘Do NOT assume that tx.origin will continue to be usable or meaningful.’

@scholtz
Copy link
Contributor Author

scholtz commented Jan 10, 2025

I believe that the smart contract developers should have right to choose if they want to check the real tx signer.. Its backed by multiple people above.

I dont get why you do not agree that it is legit use case for our use case. If person is willing to share his identity and that is the reason why person B gets some profit i dont have problem with this. Can you share some other example why we should not use this?

Current algod implementation just blocks our use case that only offchain dex aggregators can be used. Why do we have to discuss this? it should be easy to be implemented, and we have legit use case for it

@scholtz
Copy link
Contributor Author

scholtz commented Jan 10, 2025

https://medium.com/coinmonks/smart-contract-security-tx-origin-authorization-attack-vectors-027730ae601d

even in this document they say there are legit use cases to use it.. for example if we want to block the thieves from using our smart contract..

also in the example on how it should not be used is that they create smart contract with amount which is protected by the owner.. note that our use case is not like that.. we just set the fee in range 50% to 100% percent on the engagement of the verified person.. there is not much loss if person pays 50% of the fee, and if person is using illicit practice he may be banned forever

Vitalik: ‘Do NOT assume that tx.origin will continue to be usable or meaningful.’

this is from Jan 21, 2016.. its still there and ethereum for the 8 years have came through some development.. They will surly not remove it any time soon.

please enable tx.originSender

@jannotti
Copy link
Contributor

jannotti commented Jan 10, 2025

even in this document they say there are legit use cases to use it.. for example if we want to block the thieves from using our smart contract..

The article says:

For example, if one wanted to deny external contracts from calling the current contract, they could implement a require of the form require(tx.origin == msg.sender).

That is already possible by checking that CallerApplicationID is 0. So the only legitimate case described is already possible.

If you instead meant to ban specific user accounts, that would be pointless. Blockchains allow trivial Sybil style accounts. The only sane access control is allow-based, not deny-based.

I'm not in favor of adding a feature that will almost certainly be misused, leading to the many problems that Ethereum has seen with this feature. This is also why we don't allow recursive contract calls. It's too easy to screw up. That does not make Ethereum "better", it means we learned from their mistake.

I believe the proper implementation of the feature you describe is to have an optional parameter "OnBehalfOf". If and only if the sender is a DEX you explicitly trust, that parameter can be used to get the special treatment you desire. I see no downside to this approach. You might be sad that it requires you to explicitly enable DEXs, but I claim that is the only way your feature can avoid being tricked. I'm not interested in helping people write incorrect apps.

@scholtz
Copy link
Contributor Author

scholtz commented Jan 10, 2025

The only sane access control is allow-based, not deny-based.

Biatec identity solution allows the allow-based access control according to verification class, and also deny based.. the information is stored in box and can be fetched by any smart contract.. We can put to the box information that there are stolen assets on specific account and efficiently block them in our smart contracts.

OnBehalfOf

I cannot just trust the parameter onBeahlfOf.. anybody can put there anything.. Or can you please write more how to do it securely? I dont want each app call to pay few more txs fee just to verify the identity of the top level signer if it can be just one parameter in the tx object.. this will also complicate whole things and passing this data through the 3rd level smart contracts is really not feasable..

please enable tx.originSender and document the bad use cases please

@scholtz
Copy link
Contributor Author

scholtz commented Jan 11, 2025

OnBehalfOf

I do not trust anybody that would fill this information correctly.

I dont want to create indexer of smart contracts who i should trust

I dont want to pay a lot of tx fees just to verify the identity as i want our products to be competitive to traditional AMMs

I want the contract to be composable and anybody could do whatever they want in the middle.. if this will lead to custodian solutions or some solutions where on behalf the identity of one person others can trade, its ok for me.. This will not lead to any theft of funds as we dont have any use case where we would release funds on this check.

I want to know who really has the private keys to the account that initiated the app calls, and he is responsible for whatever he calls

please enable tx.originSender and document the bad use cases please

@gmalouf
Copy link
Contributor

gmalouf commented Jan 13, 2025

We've decided not to take this on at this time for the reasons @jannotti laid out above. We understand there is disagreement on this; but this is the decision right now.

@gmalouf gmalouf closed this as completed Jan 13, 2025
@gmalouf gmalouf closed this as not planned Won't fix, can't repro, duplicate, stale Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature-request Feature request that needs triage
Projects
None yet
Development

No branches or pull requests

5 participants