Skip to content

Allow call option writers to bid on spread#3

Merged
regynald merged 4 commits intomainfrom
regynald/hook-805-call-options-allow-option-writer-to-bid
May 3, 2022
Merged

Allow call option writers to bid on spread#3
regynald merged 4 commits intomainfrom
regynald/hook-805-call-options-allow-option-writer-to-bid

Conversation

@regynald
Copy link
Contributor

If the strike price of a call option is 1000 wei and the current high bid (from non option writer) is 1001 wei the option writer would only need to bid 2 wei to become the new high bidder.

This PR adds that functionality for bidding and logic for settling. Also tests for this.

@regynald regynald requested a review from jake-nyquist April 25, 2022 02:07
@linear
Copy link

linear bot commented Apr 25, 2022

HOOK-805 [call options] allow option writer to bid using only the spread

The option writer currently must put up the full amount of ETH to make a settlement bid. This would allow them to bid using only the difference between the strike and their spread. It also requires that all points where eth is distributed are aware of this (possible refactoring).

Copy link
Contributor

@jake-nyquist jake-nyquist left a comment

Choose a reason for hiding this comment

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

I think we should discuss the following alternative method:

 if bidder == writer:
    _writerBid();
 else:
    _generalBid();

....

factoring out methods to return the previous high bidders funds & to distribute the assets correctly.

....

Comment on lines 272 to 279
uint256 unnormalizedHighBid = call.bid;
if (call.highBidder == call.writer) {
unnormalizedHighBid -= call.strike;
}

// return current bidder's money
(bool sent, ) = call.highBidder.call{value: call.bid}("");
(bool sent, ) = call.highBidder.call{value: unnormalizedHighBid}("");
require(sent, "Failed to send Ether");
Copy link
Contributor

Choose a reason for hiding this comment

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

my sense is that this string of logic (+ the previous logic) may have enough complexity to just be factored out into _acceptBidFromNewBidder and _returnBidToPreviousBidder


) = call.writer.call{value: call.strike}("");
require(writerSent, "Failed to send Ether to option writer");
// If the option writer is the high bidder then they don't recieve the strike and only the underlying asset
Copy link
Contributor

Choose a reason for hiding this comment

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

... they don't receive [sp] the strike because, when they place a bid, they only paid the spread.

);
}

function testWriterCanOutbidOnSpread() public {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to see a test for these cases:

  • writer gets outbid again themselves
  • settlement happens in each case (writer bids first, writer bids last, writer outbid again)
  • reclaiming happens in each case
  • multiple options exist in the contract at once.

Copy link
Contributor

Choose a reason for hiding this comment

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

in particular, I think testing the reclaim logic is important here. Even if it works now I'm concerned that future changes might be breaking for the reclaims.

@jake-nyquist
Copy link
Contributor

nit: in the future, would be good to do the PR for removing all of the .gitignored files separate from this PR.

@regynald regynald force-pushed the regynald/hook-805-call-options-allow-option-writer-to-bid branch from b29eeb9 to 4f90328 Compare May 3, 2022 21:49
ReentrancyGuard,
Initializable
Initializable,
Test
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove Test here

import "./interfaces/IHookProtocol.sol";
import "./interfaces/IWETH.sol";

import "forge-std/Test.sol";
Copy link
Contributor

Choose a reason for hiding this comment

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

remove test here

@jake-nyquist
Copy link
Contributor

lgtm

@regynald regynald merged commit 02cd9c6 into main May 3, 2022
@regynald regynald deleted the regynald/hook-805-call-options-allow-option-writer-to-bid branch May 4, 2022 04:06
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.

2 participants