IntersectMBO / govtool

🔩 GovTool and utilities monorepo.
https://preview.gov.tools
Apache License 2.0
10 stars 7 forks source link

[BUG] Inconsistent Display of Delegated Voting Power on 'Abstain' or 'No Confidence' Votes" #1097

Open kneerose opened 3 months ago

kneerose commented 3 months ago

Context & versions

Steps to reproduce

Actual behaviour

delegateOwnVotingPower

delegateOwnVotingPower1

pmbinapps commented 3 months ago

There is such update @MSzalowski @Sworzen1. Looks like the voting power is more update after this change.

MSzalowski commented 3 months ago

It might be cause by the recalculation of the voting power on the chain. On frontend we display that value directly from the backend (without any parsing on the FE side). And that value on the backend is taken directly from DB-sync.

That might not be the issue that we on the govtool side are able to handle, but please @jankun4 could you have a look if there is some part missing?

jankun4 commented 3 months ago

The delegated amount is strictly related to the amount of Ada at the user's wallet. Each transaction (including delegating to Abstain) requires paying transaction fee which in turn lowers user's ada-holder voting power a bit.

kneerose commented 1 month ago

@MSzalowski any update on this ?

MSzalowski commented 1 month ago

As I previously and @jankun4 in a recent comment describe - the voting power is calculated directly by db-sync and we don't have any logic around this.

The difference between these values is because of fee (as @jankun4 described) and the reason why it is not displayed right after the transaction is that at this moment the value isn't calculated.

I think that we could close this issue as GovTool has nothing to do around this topic.

What do you think @kneerose ?

spannercode commented 1 month ago

@kneerose and I just had a discussion about this issue. Because of the occasional mismatch in voting power, the tests has become flaky for us. We can understand the reasoning that this value is coming from DbSync. But from the user perspective, this difference could be confusing, and should be addressed. (Maybe an update the copy that the value shown could be slightly different).

We can close the issue here as issue caused by the dependency, however, we recommend that this should be fixed, be it in the DbSync. We can wait until the next release of DbSync and retest it again. If the issue exists, then I think we should create an issue on DbSync repo so devs there can debug and fix the issue.

Ryun1 commented 4 weeks ago

I believe this issue is because;

This is a tricky one, because at any time a wallet's voting power may change The user may send or receive ada at any time (regardless of GovTool) So I feel it would be the right thing to extra logic to Govtool to preempt the cost of transaction fees

@kneerose can you point me to the test case that this is effecting please for me, it might make more sense to change the testcase rather than anything else

kneerose commented 4 weeks ago

@Ryun1

This is the Allure report. You can view the history for tests 2U and 2V there to check for flakiness. Additionally, see the overview to understand how each test works step by step.

MSzalowski commented 3 weeks ago

So I feel it would be the right thing to extra logic to Govtool to preempt the cost of transaction fees

Can we expect a scenario in which the value calculated by GovTool will differ from the final value anyway? Even if we know all the values needed for such a calculation, can we guarantee that these values will not vary?

@Ryun1 I wouldn't go for the solution where some of the voting powers (or some other ADA values) are taken from db-sync (calculated there) and some others are calculated by GovTool at least, not as a solution for flaky tests. We should have single source of truth that emits events if values changes

Ryun1 commented 3 weeks ago

oops! I wrote the opposite of what I meant

So I feel it would NOT be the right thing to extra logic to Govtool to preempt the cost of transaction fees

bosko-m commented 1 week ago

@MSzalowski @Ryun1 Do we have an understanding of whats needed here?