[Bug] Total Donations Received should display total donations received#527

decentralauren commented 5 months ago

"Total donations received" should be a sum of "Total Paid out" and "Current Pool." In the example screen shot the total amount should be 611,007.95

Screenshot 2024-02-01 at 12 10 39 PM

krisbitney commented 5 months ago

@decentralauren There are at least two reasons for the discrepancy

Extra Transfers

That particular pool received 600,000 G$ in a transfer.

See: https://explorer.celo.org/mainnet/tx/0x2d7ab81d80cecc3aacc4ee7ceb5628288db2298e4ac756b79cc4228b0fa61838

Multiple other pools also have extra G$ transfers.

Hidden Fees

I was also originally using supportSingleBatch instead of supportSingleTransferAndCall. The supportSingleBatch method has a hidden fee that I didn't know about until today.

I have donated G$ 310 to this collective. G$100 was donated using supportSingleTransferAndCall, while the remaining G$210 was donated using supportSingleBatch. Notice that there is a fee of exactly 10% for using supportSingleBatch. The Current Pool amount is only G$ 279. But the fee is not reflected in the Total Donations Received because it is not deducted from the supporter's contribution amount in the smart contract. https://explorer.celo.org/mainnet/address/0xF0E351a9B7C0e89340F543B377f9Ac7810fF7Fa0/token-transfers#address-tabs

Do other support flow methods have fees associated with them?

decentralauren commented 5 months ago

OK @krisbitney thank for clarifying - so it sounds like the 2 issues are:

current UI only shows pool contributions made in the UI
Pools have transaction fees that are not shown here

Can you please update the Total Donations Received to indeed show total cumulative amount donated to the pool (as designed)? We will need to open a ticket (assigned to us) for to show the fees paid out from the pool.

krisbitney commented 5 months ago

Yes, I can use the sum of Current Pool and Total Paid Out. This would be the total G$ the pool contract received for any reason. The total donations listed in places like the "See All Donors" page will not sum to the same amount.

decentralauren commented 5 months ago

Thanks @krisbitney This is great.

The total donations listed in places like the "See All Donors" page will not sum to the same amount.

This would only show donations made through the interface, then? Why would it not show any donations made to the contract?

krisbitney commented 5 months ago

Thanks @krisbitney This is great.

The total donations listed in places like the "See All Donors" page will not sum to the same amount.

This would only show donations made through the interface, then? Why would it not show any donations made to the contract?

This thread is the first time it has been mentioned that you want to count all token transfers to a smart contract as donations, rather than the donations explicitly tracked by the smart contracts and Subgraph database. As a result, the UI is set up to integrate the smart contracts and Subgraph database directly.

Reworking the database integration and supplementing it with a mechanism to track the sources of all transfers to the smart contract is not a trivial task. Those transfers occur in the G$ ERC20 contract. We would need to watch and filter all transactions that occur in that contract, or perhaps set up another Subgraph for it. We would then need to prevent double counting between transferred tokens and donations received through the pool contracts.

You suggested before that I didn't implement the "Total Donations Received" feature "as designed". Could you please point me to the design? When I look at the project requirements, I don't see the property mentioned. And I don't see notes about it in the Figma.

krisbitney commented 5 months ago

This PR replaces the sum of individual donations with the sum of "Total Paid Out" and "Current Pool": https://github.com/GoodDollar/GoodCollective/pull/160

decentralauren commented 5 months ago

@krisbitney - thank you for clarifying why this was implemented this way. There was unfortunately never a discussion (that I am aware of) or question about whether this value should be called from the UI or directly from the contract. In our initial walkthrough we asked that all values possible should be read directly from the contract. It seems that this may have been lost or not clearly conveyed in the game of telephone.

I'm referring to the requirements that were shared via the Figma, which clearly shows the total donations as the sum of the two numbers below

krisbitney commented 5 months ago

To clarify, all of the values come from the contracts. The UI sends events to the contracts, through transactions. The Subgraph database reads events from the contracts. The UI reads data from the Subgraph.

Write: UI --> contracts Read: contracts --> subgraph --> UI

But it is important to realize that someone can send tokens of any kind to the pool contracts without calling the smart contract functions that cause the contract to document the token transfer as a donation.

The reason the numbers don't sum nicely in the UI is because the "Total Donations Received" was being calculated by summing all of the donations known to the smart contract. But the smart contract doesn't track every token transfer. Token transfers can happen without interacting with the pool's smart contract.

I see that the numbers sum nicely in the Figma design, but that does not imply that they always would with real data. I assumed the "Total Donations Received" should correspond to your smart contract's own data, for which we created the Subgraph database.