colonyNetwork icon indicating copy to clipboard operation
colonyNetwork copied to clipboard

Allow funds to be sent directly to domains

Open area opened this issue 1 year ago • 17 comments

This is work that I've done while working on #1285 as a prerequisite for the required functionality of domains to be able to manage and exchange tokens cross-chain. For that functionality to work, I believe this functionality (or something equivalent) is required; where such an exchange is happening cross-chain, and we can't directly interrogate balances before and after the exchange on the other chain, we need a more generic way to send tokens to a domain directly in order to do the bookkeeping correctly.

Because we can't send extra data with an ERC20 token transfer that's executed by other parties, I believe the only way to do this is have a system where each domain has a unique address that can have tokens swept from it in to the corresponding domain.

Fortunately, we can (re-)use CreateX for this. By using a contract deployment salt that's a function of the colony address and domain id, we can deploy a helper contract (DomainTokenReceiver) to a fixed address for each domain (even across chains) with simple functionality to allow the sweep to occur.

I've tried to make this as transparent as possible - the deployment, upgrading, and management of the receiving contract is intended to not require any explicit interaction from the user. The experience should be that the users send tokens to a domain-specific address (which they can get from getDomainTokenReceiverAddress on ColonyNetwork) and then claim them (via claimDomainFunds on Colony) without having to check whether the contract is already deployed and/or up-to-date.

Tagging this as ready-for-review because I want feedback, but leaving as draft because I don't want it merged yet. I think the biggest question I have hanging over this implementation is what the split between the Colony and ColonyNetwork should be for the implementation but open to feedback from any and all directions.

area avatar Sep 13 '24 14:09 area

An open question here is what should happen in terms of the colony reward pot. I think I am of the opinion that whatever it is set to should be respected.

area avatar Sep 13 '24 15:09 area

So, funds sent to the main colony address or specific domain addresses will also automatically be divided up and split amongst the respective teams set by the rewards pot? Regardless if the funds are only sent to a specific team?

arrenv avatar Sep 13 '24 15:09 arrenv

split amongst the respective teams set by the rewards pot

This isn't really describing what the reward pot does. It is a single pot, and the only configuration available related to it is the fraction of incoming funds that gets put in to it, and those funds will eventually be divided among all people who have both reputation and native tokens.

It's possible this is functionality that should be removed, but until that decision is made, anything other than respecting that configuration means that it can be totally bypassed (as instead of sending funds to a colony directly, you can send funds directly to the top-level domain).

area avatar Sep 13 '24 16:09 area

This isn't really describing what the reward pot does. It is a single pot, and the only configuration available related to it is the fraction of incoming funds that gets put in to it, and those funds will eventually be divided among all people who have both reputation and native tokens.

It's possible this is functionality that should be removed, but until that decision is made, anything other than respecting that configuration means that it can be totally bypassed (as instead of sending funds to a colony directly, you can send funds directly to the top-level domain).

Haha, yeah, just crisscrossed myself with the talk of teams.

I agree with you on both. Perhaps in needs to be rethought a little or removed, but until that time, if it is configured, it seems to make sense that it can't be bypassed.

It just does not seem intuitive that it would be the case when a colony is transferring funds to itself, but, I understand that this is how this functionality would essentially work.

arrenv avatar Sep 13 '24 16:09 arrenv

isContract is now being duplicated and could be refactored out.

EDIT: Sort-of on hold until Arren and Jack have thought more about what compromise is acceptable in terms of someone looping payments to create reputation and take over a colony.

area avatar Sep 30 '24 14:09 area

Could say that internal tokens cannot be sent directly to domains?

kronosapiens avatar Oct 01 '24 15:10 kronosapiens

Could say that internal tokens cannot be sent directly to domains?

Essentially, yes, this is what I was going to implement (or, more strictly, when such tokens are claimed, they go to the root domain) but there are concerns it will render the functionality less useful than they would like.

area avatar Oct 02 '24 10:10 area

The solution that has been decided on is to let root approve a certain amount of reputation-earning tokens to enter a domain (like ERC20). Any above the approved amount are sent to root.

The intention is that the typical in-domain swap (through the UI, at least) that would result in a domain receiving reputation-earning tokens would be a motion that requires root that makes the approval for an appropriate amount and then does the swap.

Motions being approved at the same time should be supported (or at least not break in a surprising way).

area avatar Oct 10 '24 16:10 area

As-is, this means domains can send money between each other arbitrarily (while paying a network fee), which we have prevented in the past. I don't believe it is possible to prevent this, but would like to be proven wrong.

area avatar Nov 27 '24 11:11 area

From WP page 9:

Only accounts holding the funding permission may move tokens; the rule is that they may move
tokens between any two pots in the subtree rooted in the domain in which they hold the permission.
It is the expectation that this permission will in many cases be given to an extension contract
implementing a specialized decision-making mechanism, such as the funding queue described in
Section 3.2.

The current rule is that the funding permission lets users move funds bidirectionally between any two domains in the subtree.

The new behavior would allow users to move funds unidirectionally between the domain holding the funds and any other domain in the colony.

Given that intended behaviors include a) paying arbitrary addresses out of the domain and b) allowing arbitrary addresses to send funds to any domain, I don't see how preventing a user from sending funds from a domain to any other domain is possible.

That said, I don't see this as a major issue. Given that the new affordance is only unidirectional and requires funding to have been placed in the domain by someone with the authority to do so, I think it's fine, even if unintended.

kronosapiens avatar Dec 02 '24 20:12 kronosapiens

Should this functionality work with locked tokens, @arrenv? As it stands, it does not.

area avatar Dec 05 '24 12:12 area

Should this functionality work with locked tokens, @arrenv? As it stands, it does not.

Good pickup, yes, it should work with locked tokens.

Although, I am curious how it would not be supported. It makes sense that anyone outside of the colony would not be able to transfer direct to a domain with a locked token, but that is consistent with locked token behavior and a colony could still send direct to it's own domains.

Or, is this specifically related to bridging, where the bridging contract is not able to transfer a locked token?

arrenv avatar Dec 05 '24 12:12 arrenv

a colony could still send direct to it's own domains.

It could transfer internally, but if funds were sent to a domain receving address, those addresses are not allowed to transfer tokens, and so the tokens could not be forwarded on to the colony.

area avatar Dec 05 '24 13:12 area

It could transfer internally, but if funds were sent to a domain receving address, those addresses are not allowed to transfer tokens, and so the tokens could not be forwarded on to the colony.

I see, well it would certainly be good for that to not be the case and for those domain addresses to be able to transfer locked tokens.

arrenv avatar Dec 05 '24 13:12 arrenv

I've changed the implemention, but it does unfortunately rely on updating TokenAuthority, where incredibly we've not given the colony permissions to call transferFrom on a locked token, so a new authority would need to be deployed for each of our tokens. This could potentially be done transparently when upgrading a colony, but I would caution against that because of the unreliability of determining whether a token is one of ours or not, and an incorrect match would at best cause this functionality to not work with locked tokens, and at worst would forever prevent a colony from upgrading.

Is there a reason we didn't give a colony those permissions, or was this an oversight? I could imagine that the answer to that question is legal, and not technical...

area avatar Dec 05 '24 13:12 area

I don't remember how that decision was made -- most likely it predates me -- but I would guess it just wasn't a use-case that was thought about.

Would it be possible to do a "lazy updating" of authorities based on a colony's request?

kronosapiens avatar Dec 06 '24 20:12 kronosapiens

Intending to review this again before merging it.

area avatar Jan 22 '25 14:01 area