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

swap: Incentives prices #1587

Merged
merged 5 commits into from
Jul 24, 2019
Merged

swap: Incentives prices #1587

merged 5 commits into from
Jul 24, 2019

Conversation

holisticode
Copy link
Contributor

This PR adds a consistent pricing design to swap.

The idea is that internally, swap uses an own unit of account (honey), which is then converted to a currency (e.g. wei, could be any other) when the cheque is being created.

The concept of a price oracle is introduced. In order to minimize handshakes, negotiations etc. we postulate that there would be an oracle which nodes query to get the conversion from honey to wei.

For the time being this will be a hardcoded conversion of 1:1.

swap/oracle.go Outdated Show resolved Hide resolved
swap/peer.go Show resolved Hide resolved
swap/types.go Outdated Show resolved Hide resolved
@@ -658,13 +659,13 @@ func (sp *StreamerPrices) Price(msg interface{}) *protocols.Price {
// Instead of hardcoding the price, get it
// through a function - it could be quite complex in the future
func (sp *StreamerPrices) getRetrieveRequestMsgPrice() uint64 {
return RetrieveRequestMsgPrice
return swap.RetrieveRequestMsgPrice
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we update the comment on these functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To what?

We don't have an oracle for the actual honey price per message type yet, so this function comment is still correct?

swap/prices.go Show resolved Hide resolved
swap/defaults.go Show resolved Hide resolved
swap/oracle.go Outdated Show resolved Hide resolved
swap/oracle.go Outdated Show resolved Hide resolved
}

cheque := chequeMsg.Cheque
cheque := msg.Cheque
if cheque.Contract != sp.contractAddress {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be wise to rename cheque.Contract to cheque.contractAddress or cheque.address?
Not a big issue for me, but I noticed this already in lake Balaton and had to look twice here as well what exactly cheque.Contract is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree it is confusing.

Nevertheless there is apparently a go guideline which suggests to not add the type into a variable name. Contract is of type common.Address.

Nevertheless we are not very consistent with this notion either, because Owner.address is also a common.Address - I would rather like to change that name. Any suggestion?

swap/peer.go Show resolved Hide resolved
swap/peer.go Show resolved Hide resolved
swap/peer.go Outdated Show resolved Hide resolved
swap/swap.go Outdated
@@ -40,17 +40,18 @@ import (
)

const (
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you clarify for me why these constants are in swap.go and not in defaults.go? What is the purpose of defaults.go and how do we decide when to put constants there versus in a different file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. It should all be there in defaults.go

swap/swap.go Show resolved Hide resolved
swap/peer.go Outdated Show resolved Hide resolved
swap/swap.go Outdated Show resolved Hide resolved
@holisticode holisticode merged commit 579cebb into incentives Jul 24, 2019
@holisticode holisticode mentioned this pull request Jul 29, 2019
6 tasks
holisticode added a commit that referenced this pull request Aug 9, 2019
* swap: correct price handling and reset balances

* swap: conversion to wei on cheque creation; cleanup

* swap: bug fixes and TestEmitCheque

* swap: addressed PR comments

* swap: minor fixes due to PR comments
holisticode added a commit that referenced this pull request Aug 15, 2019
* swap: correct price handling and reset balances

* swap: conversion to wei on cheque creation; cleanup

* swap: bug fixes and TestEmitCheque

* swap: addressed PR comments

* swap: minor fixes due to PR comments
holisticode added a commit that referenced this pull request Aug 20, 2019
* swap: correct price handling and reset balances

* swap: conversion to wei on cheque creation; cleanup

* swap: bug fixes and TestEmitCheque

* swap: addressed PR comments

* swap: minor fixes due to PR comments
holisticode added a commit that referenced this pull request Aug 26, 2019
* swap: correct price handling and reset balances

* swap: conversion to wei on cheque creation; cleanup

* swap: bug fixes and TestEmitCheque

* swap: addressed PR comments

* swap: minor fixes due to PR comments
holisticode added a commit that referenced this pull request Aug 27, 2019
* swap: correct price handling and reset balances

* swap: conversion to wei on cheque creation; cleanup

* swap: bug fixes and TestEmitCheque

* swap: addressed PR comments

* swap: minor fixes due to PR comments
@holisticode holisticode deleted the incentives-prices branch September 2, 2019 15:20
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.

5 participants