Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal for pay() and NewOwnerEvent tracking #21

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

frankTheTank72
Copy link
Member

to discuss

Changing pay() and newOwnerEvent
Only send message stop selling if  status on sale or auction
@@ -282,10 +285,9 @@ public void txReceived() {
if (getCurrentTxAmount() >= currentPrice) {
// Conditions match, let's execute the sale
pay(); // pay the current owner
sendMessage(getCurrentTxSender().getId(), getCurrentTxAmount(), owner.getId(),trackNewOwner);
Copy link
Collaborator

Choose a reason for hiding this comment

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

So the idea is to also send the previous owner to the tracker?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

Copy link
Member

@ohager ohager left a comment

Choose a reason for hiding this comment

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

I'm very concerned about this change, as this has a significant impact on development and the system. Accepting this change will break with the plan to roll out in the beginning of the next week. The change affects a lot of parts of the system. Probably, it's better to keep this change in the backlog and apply once we find more issues.
From my understanding this is just a minor enhancement on state reporting, rather than an issue. The moment that this proposal comes up is not chosen wisely. I'm opposed to the change...and prefer to patch the reporting on the front end, which is comparatively very easy without any significant impact.

@frankTheTank72
Copy link
Member Author

I can see your concern, but we have two parts

Part i - avoid notForSale message
We have right now this really generic in the pay() function- but we should remove it here and set it only in the accept_offer function in addition to the stopsale function.

Part Il - we are only setting the newOwnwer and Price in the message. We should also fill up accountid2 on the DB - open for any other proposal as adding it to attribute3 in the message

Part I is not impacting the portal setup at all - from my point of view - we have just less events.

Part II - adding a new attribute to the message as attribute3 - can this be ignored by the processor at the beginning ? Not clear for me - if yes we could go live without processing the new value

Avoding to create an offer when auction is running
A new standard for Liquidity Pool Contracts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants