-
Notifications
You must be signed in to change notification settings - Fork 206
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
Refactor publicAuction, rename to secondPriceAuction #1562
Conversation
8547396
to
904d5a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some questions and comments.
const assetWanted = bidSeat.getAmountAllocated('Asset', assetAtAuction.brand); | ||
assert( | ||
assetMath.isGTE(assetAtAuction, assetWanted), | ||
details`more assets were wanted ${assetWanted} than were available ${assetAtAuction}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be helpful if we had a standard way of rendering assets. Are you planning to add that at some point? Otherwise, I'm not sure this is very helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will show the brand and value, so if the value is a Nat, it is already helpful. And we have an issue to display brands and other presences better. So I think it makes sense to include now, both for the benefits now and in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think when brands are printed, it shows the structure, but nothing relevant about the brand. I can't visually distinguish 3 moola from 3 simloeans.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's right, but we are working on that. It doesn't make sense to take out logging of amounts, though, because as I mentioned, the values, if Nats, are still helpful. And, we are actively working on better printing of the brands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are actively working on better printing of the brands.
Thanks, that's the clarification I was looking for.
packages/zoe/test/unitTests/contracts/test-secondPriceAuction.js
Outdated
Show resolved
Hide resolved
packages/zoe/test/unitTests/contracts/test-secondPriceAuction.js
Outdated
Show resolved
Hide resolved
const assetWanted = bidSeat.getAmountAllocated('Asset', assetAtAuction.brand); | ||
assert( | ||
assetMath.isGTE(assetAtAuction, assetWanted), | ||
details`more assets were wanted ${assetWanted} than were available ${assetAtAuction}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think when brands are printed, it shows the structure, but nothing relevant about the brand. I can't visually distinguish 3 moola from 3 simloeans.
packages/zoe/test/unitTests/contracts/test-secondPriceAuction.js
Outdated
Show resolved
Hide resolved
packages/zoe/test/unitTests/contracts/test-secondPriceAuction.js
Outdated
Show resolved
Hide resolved
52ae62f
to
2ee019e
Compare
cf27fcb
to
55413f4
Compare
Closes #1482
The secondPriceAuction closes at the
closesAfter
deadline according to thetimerAuthority
, both specified in the terms.