Fix delegate chain of trust

[n13]

Bug Description

I noticed that the method delegate statically assigns follow-delegates, This is incorrect, it needs to store the original delegation and dynamically figure it out when voting.

For two reasons

1 - A user who delegated trust to another user needs to be able to see who they delegated to 2 - When the follow user changes their trust, they originals user trust shall also be changed

Here's a simple example

A delegates his trust to B: A ---> B

C then delegates their trust to A. But since A is delegated to B, we get this table entry: C ----> B

Now A delegates their trust to D: A ----> D

Now what should happen is that C now has delegated their votes to A, which has delegated to D, therefore C delegates to D

We could fix up voting tables any time a delegatee changes their own delegatee, but I think the best way to do this is to dynamically do it at voting time, then we don't have to deal with it, and the user can see where they were delegating to.

If that is too expensive then maybe we need to store both: One table stores C ---> A

And another table stores C ----> B (because A is delegated to B at the moment)

And this second table "curdelegates" will need to be updated whenever A changes their delegation (or any user that is a delegatee changes their delegation)

Steps / Test Case

Steps to Reproduce

1. A delegates to B
2. C delegates to A
3. A now delegates to D

Expected:

C shows as delegated to A

Actual:

C shows as delegated to B

Expected 2:

C votes with D

Actual 2:

C votes with B

Test

Maybe create a separate unit test that only tests this chain function

[tlacloc]

Hello @n13 !

I was trying to replicate the bug with the steps you mention above, but it looks like assigning a delegatee doesn't iterate as you said. I mean, changing delegates as:

A delegates to B, C delegates to A, and A delegates to D

The table shows that C delegates to A not to B or D.

That information it's stored at delegate_trust_table (deltrusts), and is stored as you propose in curdelegates.

There is a for loop in delegate that iterates in all subdelegatees, but it doesn't assign any value to user calling delegation. What it do is seeks for a cycle in the delegatees, as

A -> B, B -> C, C -> A # a cycle

But in the end we store the delegatee sent by the user. So it would be nice if you can explain further how did you find that bug so we can solve it.

On the other hand, we found a bug related to revert vote, in the function mimicrevert it only iterates once,

An example

A delegates to B B delegates to C

C reverts vote

What should happen is that all votes (directly or indirectly) delegated to C should be reverted, but it only reverts B votes because it doesn't search for B delegators and so on

[n13]

Great I’m happy to be wrong about this bug can you point me to unit tests about it ?

On Oct 8, 2021, at 2:44 AM, Erick Casanova @.***> wrote:

 Hello @n13 !

I was trying to replicate the bug with the steps you mention above, but it looks like assigning a delegatee doesn't iterate as you said. I mean, changing delegates as:

A delegates to B, C delegates to A, and A delegates to D

The table shows that C delegates to A not to B or D.

That information it's stored at delegate_trust_table (deltrusts), and is stored as you propose in curdelegates.

There is a for loop in delegate that iterates in all subdelegatees, but it doesn't assign any value to user calling delegation. What it do is seeks for a cycle in the delegatees, as

A -> B, B -> C, C -> A # a cycle

But in the end we store the delegatee sent by the user. So it would be nice if you can explain further how did you find that bug so we can solve it.

On the other hand, we found a bug related to revert vote, in the function mimicrevert it only iterates once,

An example

A delegates to B B delegates to C

C reverts vote

What should happen is that all votes (directly or indirectly) delegated to C should be reverted, but it only reverts B votes because it doesn't search for B delegators and so on

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[n13]

Revert vote should be fixed but that’s a separate ticket

On Oct 8, 2021, at 2:44 AM, Erick Casanova @.***> wrote:

 Hello @n13 !

I was trying to replicate the bug with the steps you mention above, but it looks like assigning a delegatee doesn't iterate as you said. I mean, changing delegates as:

A delegates to B, C delegates to A, and A delegates to D

The table shows that C delegates to A not to B or D.

That information it's stored at delegate_trust_table (deltrusts), and is stored as you propose in curdelegates.

There is a for loop in delegate that iterates in all subdelegatees, but it doesn't assign any value to user calling delegation. What it do is seeks for a cycle in the delegatees, as

A -> B, B -> C, C -> A # a cycle

But in the end we store the delegatee sent by the user. So it would be nice if you can explain further how did you find that bug so we can solve it.

On the other hand, we found a bug related to revert vote, in the function mimicrevert it only iterates once,

An example

A delegates to B B delegates to C

C reverts vote

What should happen is that all votes (directly or indirectly) delegated to C should be reverted, but it only reverts B votes because it doesn't search for B delegators and so on

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[IanMendozaJaimes]

Sure @n13, here is the test: https://github.com/JoinSEEDS/seeds-smart-contracts/blob/f4afad30ef298ef5af280c0bbeb741ae43275caa/test/proposals.test.js#L2116

Here is the specific assert that is checking the chain of delegators is been affected: https://github.com/JoinSEEDS/seeds-smart-contracts/blob/f4afad30ef298ef5af280c0bbeb741ae43275caa/test/proposals.test.js#L2316