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

Commit

Permalink
removed TODOs which probably should not be fixed before merge (#1637)
Browse files Browse the repository at this point in the history
* removed TODOs which probably should not be fixed before merge

* more TODOs removed

* removed one more TODO

* removed more TODOs

* after review holisticode

* referenced issue at put back TODO

* swap: put in-code TODO back and changed position of NOTE

* swap: restore 2 comments in handleEmitChequeMsg function
  • Loading branch information
Eknir authored and holisticode committed Aug 27, 2019
1 parent fcbae82 commit ae43ed3
Show file tree
Hide file tree
Showing 7 changed files with 4 additions and 22 deletions.
2 changes: 0 additions & 2 deletions contracts/swap/swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ var (
type Backend interface {
bind.ContractBackend
TransactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error)
//TODO: needed? BalanceAt(ctx context.Context, address common.Address, blockNum *big.Int) (*big.Int, error)
}

// Deploy deploys an instance of the underlying contract and returns its `Contract` abstraction
Expand Down Expand Up @@ -91,7 +90,6 @@ type Params struct {

// ValidateCode checks that the on-chain code at address matches the expected swap
// contract code.
// TODO: have this as a package level function and pass the SimpleSwapBin as argument
func ValidateCode(ctx context.Context, b bind.ContractBackend, address common.Address) error {
codeReadFromAddress, err := b.CodeAt(ctx, address, nil)
if err != nil {
Expand Down
6 changes: 2 additions & 4 deletions swap/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@ const (
DefaultPaymentThreshold = 1000000
DefaultDisconnectThreshold = 1500000
// DefaultInitialDepositAmount is the default amount to send to the contract when initially deploying
// TODO: deliberate value for now; needs experimentation
// NOTE: deliberate value for now; needs experimentation
DefaultInitialDepositAmount = 0

deployRetries = 5
deployRetries = 5
// delay between retries
deployDelay = 1 * time.Second
// Default timeout until cashing in cheques is possible - TODO: deliberate value, experiment
// Should be non-zero once we implement waivers
defaultCashInDelay = uint64(0)
// This is the amount of time in seconds which an issuer has to wait to decrease the harddeposit of a beneficiary.
Expand Down
1 change: 0 additions & 1 deletion swap/oracle.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ type PriceOracle interface {
}

// NewPriceOracle returns the actual oracle to be used for discovering the price
// TODO: Add a config flag so that this can be configured via command line
// For now it will return a default one
func NewPriceOracle() PriceOracle {
return &FixedPriceOracle{
Expand Down
9 changes: 2 additions & 7 deletions swap/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ func (sp *Peer) handleMsg(ctx context.Context, msg interface{}) error {

// handleEmitChequeMsg should be handled by the creditor when it receives
// a cheque from a debitor
// TODO: validate the contract address in the cheque to match the address given at handshake
// TODO: this should not be blocking
func (sp *Peer) handleEmitChequeMsg(ctx context.Context, msg *EmitChequeMsg) error {
cheque := msg.Cheque
log.Info("received cheque from peer", "peer", sp.ID().String())
Expand Down Expand Up @@ -108,14 +106,12 @@ func (sp *Peer) handleEmitChequeMsg(ctx context.Context, msg *EmitChequeMsg) err

receipt, err = otherSwap.CashChequeBeneficiary(opts, sp.backend, sp.swap.owner.Contract, big.NewInt(int64(actualAmount)))
if err != nil {
//TODO: do something with the error
// TODO: do something with the error
// and we actually need to log this error as we are in an async routine; nobody is handling this error for now
log.Error("error cashing cheque", "err", err)
return
}
log.Debug("cash tx mined", "receipt", receipt)
//TODO: after the cashCheque is done, we have to watch the blockchain for x amount (25) blocks for reorgs
//TODO: make sure we make a case where we listen to the possibility of the peer shutting down.
log.Info("Cheque successfully submitted and cashed")
}()
return err
Expand Down Expand Up @@ -143,7 +139,7 @@ func (sp *Peer) processAndVerifyCheque(cheque *Cheque) (uint64, error) {

if err := sp.saveLastReceivedCheque(cheque); err != nil {
log.Error("error while saving last received cheque", "peer", sp.ID().String(), "err", err.Error())
// TODO: what do we do here?
// TODO: what do we do here? Related issue: https://github.com/ethersphere/swarm/issues/1515
}

return actualAmount, nil
Expand Down Expand Up @@ -190,7 +186,6 @@ func verifyChequeAgainstLast(cheque *Cheque, lastCheque *Cheque, expectedAmount
actualAmount -= lastCheque.Amount
}

// TODO: maybe allow some range around expectedAmount?
if expectedAmount != actualAmount {
return 0, fmt.Errorf("unexpected amount for honey, expected %d was %d", expectedAmount, actualAmount)
}
Expand Down
1 change: 0 additions & 1 deletion swap/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ func TestEmitCheque(t *testing.T) {
t.Fatalf("Expected exactly one cheque at creditor, but there are %d:", len(creditorSwap.cheques))
}
*/

}

// TestTriggerPaymentThreshold is to test that the whole cheque protocol is triggered
Expand Down
3 changes: 0 additions & 3 deletions swap/swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ func (s *Swap) sendCheque(peer enode.ID) error {
s.cheques[peer] = cheque

err = s.store.Put(sentChequeKey(peer), &cheque)
// TODO: error handling might be quite more complex
if err != nil {
return fmt.Errorf("error while storing the last cheque: %s", err.Error())
}
Expand All @@ -230,7 +229,6 @@ func (s *Swap) sendCheque(peer enode.ID) error {
}

// reset balance;
// TODO: if sending fails it should actually be roll backed...
err = s.resetBalance(peer, int64(cheque.Amount))
if err != nil {
return err
Expand Down Expand Up @@ -394,7 +392,6 @@ func (s *Swap) GetParams() *swap.Params {

// Deploy deploys a new swap contract
func (s *Swap) Deploy(ctx context.Context, backend swap.Backend, path string) error {
// TODO: What to do if the contract is already deployed?
return s.deploy(ctx, backend, path)
}

Expand Down
4 changes: 0 additions & 4 deletions swap/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ import (
"github.com/ethereum/go-ethereum/common"
)

// TODO: add handshake protocol where we exchange last cheque (this is useful if one node disconnects)
// FIXME: Check the contract bytecode of the counterparty

// ChequeParams encapsulate all cheque parameters
type ChequeParams struct {
Contract common.Address // address of chequebook, needed to avoid cross-contract submission
Expand All @@ -34,7 +31,6 @@ type ChequeParams struct {
}

// Cheque encapsulates the parameters and the signature
// TODO: There should be a request cheque struct that only gives the Serial
type Cheque struct {
ChequeParams
Signature []byte // signature Sign(Keccak256(contract, beneficiary, amount), prvKey)
Expand Down

0 comments on commit ae43ed3

Please sign in to comment.