all: first working SWAP version by mortelli · Pull Request #1554 · ethersphere/swarm

mortelli · 2019-07-11T16:04:52Z

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.

swap/peer.go

contracts/swap/swap.go

p2p/protocols/accounting.go

swap/peer.go

swap/protocol_test.go

swarm.go

swarm_test.go

swap/peer.go

swap/protocol.go

zelig

jusy some preliminary comments as i went through quickly

cmd/swarm/run_test.go

contracts/swap/swap.go

swap/oracle.go

swap/swap.go

swap/types.go

swarm.go

acud

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:

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).
Probably unnecessary use of pointer semantics in some cases (do you need to change params on-the-fly?)
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
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
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

cmd/swarm/run_test.go

contracts/swap/swap.go

contracts/swap/contract/simpleswap.go

contracts/swap/swap.go

swarm.go

testutil/dummy.go

contracts/swap/swap.go

api/config.go

contracts/swap/swap.go

swap/cheque.go

swap/peer.go

swap/swap.go

swap/types.go

testutil/dummy.go

…1768d#r307936535

* 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

…ivedCheque function

…all tests

* swap: fix race condition in tests

* '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)

mortelli requested a review from nonsense July 11, 2019 16:04

nonsense reviewed Jul 11, 2019

View reviewed changes

swap/peer.go Outdated Show resolved Hide resolved

nonsense reviewed Jul 11, 2019

View reviewed changes

swap/peer.go Outdated Show resolved Hide resolved