Allow funds to be sent directly to domains #1291
Conversation
|
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. |
|
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? |
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. |
area
force-pushed
the
feat/fund-domains-directly
branch
from
0eb11bc
to
9575ac7
Compare
area
force-pushed
the
feat/fund-domains-directly
branch
4 times, most recently
from
d698cfc
to
d5c36c3
Compare
| return domainReceiverResolverAddress; | ||
| } | ||
|
|
||
| function checkDomainTokenReceiverDeployed( |
There was a problem hiding this comment.
Can we choose a different naming convention for this type of idempotent deployment? I feel the same about deployCreateXIfNeeded. I'd be happy rolling this up into getDomainTokenReceiverAddress with a dev note that it deploys the contract idempotently if needed. idempotentDeployDomainTokenReceiver goes in that direction but is just as wordy so I don't know if that's a huge win. Open to your thoughts on this.
There was a problem hiding this comment.
I'd be happy rolling this up into getDomainTokenReceiverAddress with a dev note that it deploys the contract idempotently if needed.
I'm quite against this, as it makes it significantly less intuitive to use from the frontend side (as it's no longer a view function).
In terms of naming... I agree that it's much of a muchness. I will say I'm completely unfussed about deployCreateXIfNeeded though, as it's a non-contract function only used in tests.
EDIT: I'm on board for making things consistent, at least.
There was a problem hiding this comment.
Ok, how about idempotentDeployCONTRACTNAME for these types of things? So idempotentDeployCreateX and idempotentDeployDomainTokenReceiver?
area
force-pushed
the
feat/fund-domains-directly
branch
2 times, most recently
from
f8120c9
to
c978a65
Compare
area
force-pushed
the
feat/fund-domains-directly
branch
from
c978a65
to
569309e
Compare
| @@ -106,6 +107,38 @@ contract ColonyFunding is | |||
| emit ColonyFundsClaimed(msgSender(), _token, feeToPay, remainder); | |||
| } | |||
|
|
|||
| function claimDomainFunds(address _token, uint256 _domainId) public stoppable { | |||
| require(domainExists(_domainId), "colony-funding-domain-does-not-exist"); | |||
| address domainTokenReceiverAddress = IColonyNetwork(colonyNetworkAddress) | |||
There was a problem hiding this comment.
Thinking more about whether this deployment should go on the network or the colony, while putting it on the colony seems preferable to avoid the external call, if we're going to check the resolver each time then the external call will be inevitable, and if so probably best to keep all the deployment logic in one place.
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
getDomainTokenReceiverAddressonColonyNetwork) and then claim them (viaclaimDomainFundsonColony) without having to check whether the contract is already deployed and/or up-to-date.Tagging this as
ready-for-reviewbecause 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 theColonyandColonyNetworkshould be for the implementation but open to feedback from any and all directions.