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

Investigate events emitted when selling canvas #16

Closed
krisu-pl opened this issue Apr 28, 2018 · 5 comments
Closed

Investigate events emitted when selling canvas #16

krisu-pl opened this issue Apr 28, 2018 · 5 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@krisu-pl
Copy link
Member

There seems to be something wrong with emitted events.
When accepting Sell offer, from and to addresses are the same.
When accepting Buy offer by the owner, the amount is 0 and to address is 0x0
image

@krisu-pl krisu-pl added this to the Release 1.0 milestone Apr 28, 2018
@krisu-pl
Copy link
Member Author

krisu-pl commented Apr 28, 2018

@k-misztal it may be caused because Solidity uses pointers instead of copying variables... check this out. If that's the case you might need to review the whole contract for this kind of bugs

image

@krisu-pl
Copy link
Member Author

krisu-pl commented Apr 28, 2018

Same error as above, but in function acceptSellOffer.

First, you make the sender the owner of the canvas (correct).

canvas.owner = msg.sender;
cancelSellOffer(_canvasId);

emit CanvasSold(_canvasId, msg.value, sellOffer.seller, msg.sender);

But then you execute cancelSellOffer

 require(canvas.owner == msg.sender);
 canvasForSale[_canvasId] = SellOffer(false, msg.sender, 0, 0x0);

where you change the sell offer seller to msg.sender, which is actually the buyer in this case

@krisu-pl
Copy link
Member Author

FYI moving emitting events above the critical lines solves the problem, but I'd still take a deeper look into the whole contract and other events

@krisu-pl
Copy link
Member Author

One more thing, event CanvasNoLongerForSale is our only indicator of "Sell Offer Cancelled". It shouldn't be triggered when accepting sell offer as we would have 2 events in the history (Canvas Sold & Sell Offer Cancelled). I think cancelSellOffer function should be split into two, so you can reset the sell offer but not emit the event.

@krisu-pl krisu-pl added the bug Something isn't working label Apr 28, 2018
@k-misztal
Copy link
Member

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants