Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

all: first working SWAP version #1554

Merged
merged 147 commits into from
Aug 27, 2019
Merged

all: first working SWAP version #1554

merged 147 commits into from
Aug 27, 2019

Conversation

mortelli
Copy link
Contributor

@mortelli mortelli commented Jul 11, 2019

This PR contains a first working but experimental implementation of SWAP incentive system which aims to ensure fair use of bandwidth in the Swarm network.

The system works as follows: Swarm nodes communication and data exchange is tracked and recorded into balance books. As nodes cross hardcoded balance thresholds, signed cheques are sent to the remote peers and cached out on chain through chequebook smart contracts.

This first implementation in general assumes a happy flow. Next steps will be to refine it on several aspects and to better address error scenarios.


TODO:

  • Address all TODOs, and clearly mark those that are not easy to address, so that we can discuss or create issues if necessary.
  • Implement correct balance handling and remove resetBalance
  • Clean up the SimpleSwap naming (either rename the smart contracts or refactor the contractReference.Instance)
  • Clean up the log calls to be following our style (log.<Level>(msg, key1, val1, key2, val2,...))
  • Use structured logging:
		log.Info(fmt.Sprintf("balance for peer %s is %d", peer.ID(), s.balances[peer.ID()]))

should be

		log.Info("balance for peer", "peer", peer.ID(), "balance", s.balances[peer.ID()])
  • Review logging levels - INFO vs DEBUG vs TRACE. We don't have concrete rules, but probably best to avoid using INFO for debug messages. Review ERRORs - every ERROR in our gateways nodes should be reviewed by a developer, once code goes to production. We should strive for very few/no errors.

@mortelli mortelli requested a review from nonsense July 11, 2019 16:04
swap/peer.go Outdated Show resolved Hide resolved
swap/peer.go Outdated Show resolved Hide resolved
contracts/swap/swap.go Outdated Show resolved Hide resolved
contracts/swap/swap.go Outdated Show resolved Hide resolved
swap/peer.go Outdated Show resolved Hide resolved
swap/protocol_test.go Outdated Show resolved Hide resolved
swarm.go Outdated Show resolved Hide resolved
swap/peer.go Outdated Show resolved Hide resolved
swap/peer.go Outdated Show resolved Hide resolved
swap/peer.go Outdated Show resolved Hide resolved
swap/protocol.go Outdated Show resolved Hide resolved
swap/protocol.go Outdated Show resolved Hide resolved
@mortelli mortelli marked this pull request as ready for review July 25, 2019 16:01
@mortelli mortelli changed the title Incentives api, swarm, main, chequebook, contract, swap, network, stream, protocols, state, testutil: first working SWAP version Jul 26, 2019
Copy link
Member

@zelig zelig left a comment

Choose a reason for hiding this comment

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

jusy some preliminary comments as i went through quickly

cmd/swarm/run_test.go Outdated Show resolved Hide resolved
contracts/swap/swap.go Outdated Show resolved Hide resolved
contracts/swap/swap.go Outdated Show resolved Hide resolved
contracts/swap/swap.go Outdated Show resolved Hide resolved
swap/oracle.go Outdated Show resolved Hide resolved
swap/swap.go Outdated Show resolved Hide resolved
swap/swap.go Outdated Show resolved Hide resolved
swap/swap.go Outdated Show resolved Hide resolved
swap/types.go Outdated Show resolved Hide resolved
swarm.go Outdated Show resolved Hide resolved
Copy link
Member

@acud acud left a comment

Choose a reason for hiding this comment

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

so... first pass from my side. there's a lot of code here. I'm not a big fan of these huge PRs and I am still adamant that this could have been broken into smaller pieces of work that are easier to reason about and that could be merged directly into master.
Comments in a nutshell are:

  1. lots of aesthetic changes that need to be done, mostly in the realms of logging and error handling (i.e. dont log errors then return them).
  2. Probably unnecessary use of pointer semantics in some cases (do you need to change params on-the-fly?)
  3. Passing private keys around - I am not a big fan of this. I'd rather see this functionality in some core component that handles the keys
  4. You have generated bindings but the solidity files are not here - why? and where's the tooling to keep both on par? I suggest you add the solidity files to be in the same repository
  5. Some unwanted full-database-scan functionality on the state store - I explicitly don't like it. you should know what you're querying for. and if you don't - you should at least construct correct indexes that would allow you to iterate over the keys you KNOW you want to have. leveldb is not a SQL database. please bear that in mind, and by the looks of it - the usage of that functionality might cause us trouble. If you're unsure on what is being requested of you - please ask and we will kindly guide you to the correct solution.

api/config.go Outdated Show resolved Hide resolved
cmd/swarm/run_test.go Outdated Show resolved Hide resolved
contracts/swap/swap.go Outdated Show resolved Hide resolved
contracts/swap/contract/simpleswap.go Show resolved Hide resolved
contracts/swap/swap.go Outdated Show resolved Hide resolved
swarm.go Outdated Show resolved Hide resolved
swarm.go Outdated Show resolved Hide resolved
swarm.go Outdated Show resolved Hide resolved
testutil/dummy.go Outdated Show resolved Hide resolved
testutil/dummy.go Outdated Show resolved Hide resolved
@holisticode holisticode changed the title api, swarm, main, chequebook, contract, swap, network, stream, protocols, state, testutil: first working SWAP version all: first working SWAP version Jul 29, 2019
api/config.go Outdated Show resolved Hide resolved
contracts/swap/swap.go Show resolved Hide resolved
contracts/swap/swap.go Outdated Show resolved Hide resolved
swap/cheque.go Show resolved Hide resolved
swap/peer.go Outdated Show resolved Hide resolved
swap/swap.go Outdated Show resolved Hide resolved
swap/swap.go Outdated Show resolved Hide resolved
swap/swap.go Outdated Show resolved Hide resolved
swap/types.go Outdated Show resolved Hide resolved
testutil/dummy.go Outdated Show resolved Hide resolved
holisticode and others added 16 commits August 27, 2019 12:45
* swap: add getLastChequeValues function

* swap: iterate getLastChequeValues function

* swap: refactor createCheque

* swap: update comment for cheque Sign function

* swap: update cheque Sign function comment

* swap: add err check in getLastChequeValues function
* swap: refactor createCheque and sendCheque functions to receive *Peer variables instead of IDs

* swap: rename peerID variables to peer for consistency in swap.go

* swap: change error message when failing to get peer in Add function, simplify swapPeer.Peer.ID() calls
* swap: fix race condition in tests
@mortelli mortelli merged commit 8e8aefd into master Aug 27, 2019
@skylenet skylenet added this to the 0.5.0 milestone Sep 17, 2019
chadsr added a commit to chadsr/swarm that referenced this pull request Sep 23, 2019
* 'master' of github.com:ethersphere/swarm:
  pss: Modularize crypto and remove Whisper. Step 1 - isolate whisper code (ethersphere#1698)
  pss: Improve pressure backstop queue handling - no mutex (ethersphere#1695)
  cmd/swarm-snapshot: if 2 nodes to create snapshot use connectChain (ethersphere#1709)
  network: Add API for Capabilities (ethersphere#1675)
  pss: fixed flaky test that was using a global variable instead of a local one (ethersphere#1702)
  pss: Port tests to `network/simulation` (ethersphere#1682)
  storage: fix hasherstore seen check to happen when error is nil (ethersphere#1700)
  vendor: upgrade go-ethereum to 1.9.2 (ethersphere#1689)
  bzzeth: initial support for bzz-eth protocol (ethersphere#1571)
  network/stream: terminate runUpdateSyncing on peer quit (ethersphere#1696)
  all: first working SWAP version (ethersphere#1554)
  version: update to v0.5.0 unstable (ethersphere#1694)
  chunk, storage: storage with multi chunk Set method (ethersphere#1684)
  chunk, storage: add HasMulti to chunk.Store (ethersphere#1686)
  chunk, shed, storage: chunk.Store GetMulti method (ethersphere#1691)
  api, chunk: progress bar support (ethersphere#1649)
@ralph-pichler ralph-pichler deleted the incentives branch November 11, 2019 12:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet