Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

staking: check caller is the channel proxy when settling #203

Merged
merged 3 commits into from
May 28, 2020

Conversation

abarmat
Copy link
Contributor

@abarmat abarmat commented May 21, 2020

Issue:

The settle() function needs to be public so the multisig can call it.
It means that it can be called by any party, and as the channelID is also public (advertised in allocate()) you could be settling a channel for someone else.

Changes:

  • Pass the channel proxy address in the allocate() call.
  • Keep a link of the channel proxy address to the channel identifier.
  • Verify from settle() that the caller is a channel proxy.
  • Remove channelID from settle() and instead use the caller (proxy address) to find the allocation.

Changes required in agents:

  • Calculate the Channel Proxy Multisig address in the client to pass to allocate(). Connext will provide a helper function to do this.

@abarmat abarmat changed the base branch from master to ariel/staking-tests-1 May 21, 2020 19:19
@abarmat
Copy link
Contributor Author

abarmat commented May 21, 2020

Most of the changes in disputes.js seems to be prettier.
Focus on general.test.js and staking.sol for the review.

@abarmat abarmat requested a review from davekaj May 21, 2020 20:13
@abarmat abarmat self-assigned this May 21, 2020
@abarmat abarmat force-pushed the ariel/staking-tests-1 branch 2 times, most recently from c309d50 to f2140ad Compare May 25, 2020 16:01
Base automatically changed from ariel/staking-tests-1 to master May 25, 2020 19:04
@abarmat abarmat changed the base branch from master to ariel/staking-alloc-price May 26, 2020 00:26
@abarmat abarmat force-pushed the ariel/staking-check-caller branch from fa80fb3 to 9e815dc Compare May 26, 2020 00:33
@abarmat
Copy link
Contributor Author

abarmat commented May 26, 2020

Rebased

Copy link
Contributor

@davekaj davekaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small change requested, can be merged either way


// Receive funds from the channel multisig
// We use channelID to find the indexer owner of the channel
require(isChannel(_channelID), "Channel: does not exist");
require(isChannel(channelID), "Channel: does not exist");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be moved above the deletion of channelsProxy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do and push the changes

@abarmat abarmat merged commit c780caf into ariel/staking-alloc-price May 28, 2020
@abarmat abarmat deleted the ariel/staking-check-caller branch May 28, 2020 14:44
abarmat added a commit that referenced this pull request May 28, 2020
* staking: advertise, in the AllocationCreated event, the non-binding price charged for queries related to an allocation
* staking: check caller is the channel proxy when settling (#203)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants