Enforce fair outcome in direct and virtual fund objectives #1610
Conversation
👷 Deploy Preview for nitrodocs processing.
|
✅ Deploy Preview for nitro-gui canceled.
|
✅ Deploy Preview for nitro-storybook canceled.
|
There was a problem hiding this comment.
I think I'm on board with restricting virtual channels in the way you have here.
I am far less sure about the ledger channel restriction. Why do you think it is important for each party in the ledger channel to have the same initial allocation (i.e. for there to be a uniform distribution)?
| // hasEqualOutcome returns true if the outcome allocates an equal amount to each participant | ||
| func hasEqualOutcome(s state.State, me types.Address) bool { |
There was a problem hiding this comment.
hasUniformOutcome is better, I think?
| if !hasEqualOutcome(initialState, myAddress) { | ||
| return Objective{}, errors.New("attempted to initiate new direct-funding objective with non-equal outcome") | ||
| } |
There was a problem hiding this comment.
Follow on changes here if you like "uniform outcome".
|
|
||
| request.Outcome[0].Allocations[0].Amount = big.NewInt(10) | ||
| if _, err := NewObjective(request, false, testState.Participants[0], big.NewInt(TEST_CHAIN_ID), getByParticipant, getByConsensus); err == nil { | ||
| t.Errorf("Expected an error when constructing a direct fund objective with an unfair outcome") |
There was a problem hiding this comment.
| t.Errorf("Expected an error when constructing a direct fund objective with an unfair outcome") | |
| t.Errorf("Expected an error when constructing a direct fund objective with an non-uniform outcome") |
| for i, a := range e.Allocations { | ||
| if i == payments.PAYER_INDEX && a.Amount.Cmp(total) != 0 { | ||
| return false | ||
| } else if i != payments.PAYER_INDEX && a.Amount.Cmp(big.NewInt(0)) != 0 { |
There was a problem hiding this comment.
Super nit but you don't need the else here, and I believe it is encouraged to omit it (saves some indentation).
| } | ||
|
|
||
| // consensusStateSignatures prepares a consensus channel with a consensus outcome and returns the signatures on the consensus state | ||
| func consensusStateSignatures(leader, follower testactors.Actor, guarantees ...consensus_channel.Guarantee) [2]state.Signature { | ||
| return prepareConsensusChannelHelper(0, leader, follower, leader, 0, 0, 2, guarantees...).Signatures() | ||
| return prepareConsensusChannelHelper(0, leader, follower, leader, 0, 5, 2, guarantees...).Signatures() | ||
| } | ||
|
|
||
| func prepareConsensusChannelHelper(role uint, leader, follower, leftActor testactors.Actor, leftBalance, rightBalance, turnNum int, guarantees ...consensus_channel.Guarantee) *consensus_channel.ConsensusChannel { |
There was a problem hiding this comment.
Out of scope nit: I wonder if there's a more useful way to name this function. "Helper" doesn't really tell us anything...
There was a problem hiding this comment.
Out of scope but I think the virtual fund tests could use some refactoring in general. Helpers within helpers relying of specific test data made updating this code slightly tricky.
By enforcing this rule we rule out some ledger scenarios that we know we're not interested in.For example, we wouldn't want to join a ledger channel where we're the only one contributing to the balance, since that ledger channel won't be that useful for us. Is there a use case where we would want to open a ledger a channel with a non-uniform balance? |
Stand-up discussion brought up the point that depending on our role we may be interested in opening ledger channels with unequal balances. Enforcing a uniform balance seems more like an optional policy and not something that should be baked into the protcol. |
lalexgap
force-pushed
the
enforce-fair-outcome
branch
4 times, most recently
from
8b32781
to
fec0b89
Compare
|
Closing for now, will re-open when ready. |
Fixes #1608
This updates the protocols to enforce a "fair" outcome based on the some of the policy work.
payerstarts with the full balance.By enforcing these outcome restrictions in our protocols our nitro clients are protected from accepting an objective with an unfavourable or less efficient outcome.