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

contracts/swap, swap: refactor contractAddress, remove backend as inputParam, function order in contracts/swap #1748

Merged
merged 4 commits into from
Sep 16, 2019

Conversation

Eknir
Copy link
Contributor

@Eknir Eknir commented Sep 12, 2019

This PR includes a small refactor with regards to where the chequebook contractAddress is stored.

Previously, it was stored explicitly in swap.owner.Contract, while it was also implicitly stored in swap.contract (otherwise we would not be able to do calls to the contract).

This PR removes swap.owner.Contract as a place to store the contractAddress and exposes the contractAddress via swap.getParams().

Additionally, two other refactors were included:

  • remove backend as an input parameter in all swap functions which have swap as a receiver (instead, use swap.backend)
  • change the order of functions in contracts/swap, to reflect the order of the interface definition

@Eknir Eknir self-assigned this Sep 12, 2019
@Eknir Eknir changed the title Incentives contract address contracts/swap, swap: refactor contractAddress, remove backend as inputParam, function order in contracts/swap Sep 12, 2019
swap/peer.go Outdated Show resolved Hide resolved
swap/swap_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mortelli mortelli left a comment

Choose a reason for hiding this comment

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

LGTM. just one question

swap/protocol_test.go Show resolved Hide resolved
Copy link
Contributor

@santicomp2014 santicomp2014 left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -130,7 +129,7 @@ func createOwner(prvkey *ecdsa.PrivateKey) *Owner {

// DeploySuccess is for convenience log output
func (s *Swap) DeploySuccess() string {
return fmt.Sprintf("contract: %s, owner: %s, deposit: %v, signer: %x", s.owner.Contract.Hex(), s.owner.address.Hex(), s.params.InitialDepositAmount, s.owner.publicKey)
return fmt.Sprintf("contract: %s, owner: %s, deposit: %v, signer: %x", s.GetParams().ContractAddress.Hex(), s.owner.address.Hex(), s.params.InitialDepositAmount, s.owner.publicKey)
Copy link
Contributor

@holisticode holisticode Sep 13, 2019

Choose a reason for hiding this comment

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

Not related to this PR, but your change reminds me of it: signer: %x........,s.owner.publickey actually prints garbage. Can you open an issue where we'd print the hex value of it? Thanks.

As it is a very small change, you could do it as part of this PR if you want.

Copy link
Contributor

@holisticode holisticode left a comment

Choose a reason for hiding this comment

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

Very nice PR, it brings more simplification and clarity to the actual Swap elements.

@zelig zelig merged commit 02bbb14 into master Sep 16, 2019
@skylenet skylenet added this to the 0.5.0 milestone Sep 17, 2019
@mortelli mortelli deleted the incentives-contractAddress branch November 14, 2019 18:58
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

7 participants