Modifications for ETH flow:

[josojo]

This is a small write-up for the ETH-flow modification and how I am planning to do them. Any feedback is welcome.

High level expectations on the ETH-flow orders:

• User can query all their order including the ETH-flow orders via the api
• Front-end should receive all the original order information — including the user address placing the order and the original valid_to of an order
• Solvers and other API consumers should have always the possiblity to convert an ETH-flow order received from the api to the user_order( as the order was intended by the user) and contract_order ( as the order will be executed in the smart contract)
• The specs must be compatible with the ETH-Flow contract specs

Specification:

• [ ] Part 1: Parsing order placements from smart contract events:
  • ETH-Flow event parsing:
    • Add ABI for ETHFlow contract and then use the existing EventUpdater to parse the new events
  • Event storing:
    • Create a new table ethflow_signatures_events with the fields: (block_number, log_index, order_uid, valid_to, user_addres, ...)
  • Event maintenance with reorg protection:
    • Modify replace_events() to also consider the newest blocks from ethflowsignatures in last_event_block and then use replace_events() in order to enable the existing reorg_protection for event parsing also for the ethflowsignature events
• [ ] Part 2: Storing ETH-flow orders via API (While all eth-flow order information could be fully parsed from the events, we still require the front-end to place orders via the API, in order to allow the front-end to parse the quote information)
  • Create a new Signature type: EthFlow, in order to mark Eth-flow orders uniquely. The difference between EthFlow orders and EIP1271 orders is that the owner field is different: In Eth-Flow orders, the owner will be the real user_address placing the eth-flow order, and in EIP-1271 orders, it will be the contract that is supposed to be called on a smart contract level.
  • In validate_and_construct_order() create a separate handler for the Signature::EthFlow that checks:
    • Quote is valid and recent
    • Owner is the real user placing the order on the eth_hanlder
    • Valid_to should be the real valid to specified by the user
    • Receiver is not null and not the ethflow contract
    • Signature does not need to be checked, as it can always be constructed later on from the payload
    • Approvals are not checked, as they will be set from the EthFlow contract during deployment
    • The on-chain order does not yet have to be placed! Normally we would expect that the API call for order placement happens first
  • Do not allow EIP1271 orders with the eth-flow contract as the owner. Otherwise, there might be duplication of orders.
• [ ] Part 3: Getting open orders for /auction endpoint:
  • Query only orders that we received via the API orders endpoint and for which we have a parsed ETH-flow event. This can be archived by appending two additional fields inORDERS_SELECT:

     ...,
       o.signing_scheme = ‘ethftlow’ AND (
      CASE WHEN EXISTS 
            (
         SELECT *
         FROM ethflowsignature_events p \
         WHERE o.uid = p.order_uid \
        LIMIT 1 ) \
      THEN 'true' \
      ELSE 'false' )AS eth_flow_order_pending \,
      (SELECT p.valid_to
         FROM ethflowsignature_events p \
         WHERE o.uid = p.order_uid \
        LIMIT 1 ) AS eth_flow_valid_to

And then we can use the new variables eth_flow_order_pending and eth_flow_valid_to to determine whether an order is valid and whether it should be considered

• [ ] PART 4: Settling Eth-Flow Orders:

  • Create a new function: create_eth_flow_signature that creates the signature from order data.
  • Use this new function to create the EIP1271 signature for the eth-flow
  • While encoding the settlement of an order, following modifications need to be made:
    • valid_to of the order should be set to uint(-1)
      • owner should be the eth-flow contract
      • (these two changes are the difference between the intended user order and the way order is placed by the eth flow contract, see table here )

• [ ] Part 5: User payback service: – Periodically query all placed ETH orders that are not settled from the database

  • Check whether orders have been placed with sufficient slippage by comparing to quote
  • If all checks pass, initiate transactions that trigger the sth-flow contract to refund.

• [ ] Part 6: Displaying orders to the front-end – Because the owner and the valid_to are the real ones specified by the user and are not the eth-flow contract and infinity, orders are being displayed correctly in the front-end

• [ ] Part 7: Parse on-chain order invalidation:

  • Parse invalidation events from the blockchain using the EventUpdater
  • Put the events into a new database: invalidation_orders
  • To determine the status of an eth-flow order, always query invalidated_orders database as well.

General thoughts

Generally, the challenge of the proposal is that the there are two different kinds of orders - the real order with the real intend of the user and the order created by the eth-flow contract. There are small differences (cf. here) in these order types: The difference is so small that it does not justify the effort to introduce a completely new order type, but still big enough to introduce additional complexity. In the upper spec, the complexity is shifted into "payload encoding of an order". We could also go for the second approach and really introduce a new order type or at least a new field in the order that is representing the original user placing the eth-flow order.

[MartinquaXD]

Nice write up! There is just one thing I'd like to double check: I don't know the ins and outs of the ETH flow but was the system designed such that it keeps us from wasting a bunch of gas money refunding orders which can not reasonably be settled (like from the stable to stable guys). I believe ETH <-> stETH could be such a case.

I think 👇 would force everybody to go through our quoting process, effectively preventing out of market price orders.

In validate_and_construct_order() create a separate handler for the Signature::EthFlow that checks: Quote is valid and recent

AFAICT this protects us from those malicious trades but I just wanted to make sure this was thoroughly thought through before moving on because such orders have caused a bunch of trouble lately.

[josojo]

That is an important point:

What we were planning to do:

• Always use a higher slippage requirement in the front-end: roughly 1%.
• As mentioned in Part 5: Our service only takes care of the refunds, if the limit price met the slippage requirement: 1% slippage with respect to the provided quote.
• In part 2: I was specifying a that a valid and recent quote is required. Though I decided to not check that the slippage is at least 1%. I thought that it might be valuable to still leave these orders in the system. But for sure, we will only take care of the refunds, if the slippage requirement was met. I think for these special cases of custom orders not meeting the slippage requirement, we will have a hardhat/foundry script that allows the users to claim back their tokens.

[fedgiac]

If we introduce the virtual signature type "ethflow" then there could be the same order appearing twice in the orderbook: once as the user ethflow order, and once as the ETH-flow contract eip1271 order. However, these are not two distinct orders that can be matched onchain. How do we treat this case? Should we disallow any order that is EIP1271 signer by (any of the previous and current) ETH-flow contract(s)?

(How does it work right now with EIP1271 if a contract creates two valid orders, but one becomes invalid if the other is settled?)

What about doing this: we let users create orders with the virtual signature type ethflow. This trigger the creation of two orders in the API: the "virtual" user order and the real, eip1271-signed ETH-flow order. When settling, all ethflow-type orders are ignored but they are implicitly included by including the eip1271 ones.

Also, where do we store the information that an ETH sell order was "consumed"? This could mean any of deletion, update or (partially) filled.

Comments on part 2.

Owner is the ethflow contract

Should this be the case? As they are not "real" order it would be better if the owner were the actual user who created the order. It's implicit that since the order has virtual signature type ethflow then the real owner will be the ETH-flow contract. For example, if querying all orders for an address, we probably also want to include all ETH orders by this address, even if technically owner by the ETH-flow contract.

We should also check that the buy token is not eth/weth. Here we should also check slippage I suppose. In particular in part 5:

Check whether orders have been placed with sufficient slippage by comparing to quote

Is step 5 the appropriate step to do this check? I imagine there could be no extra check needed in 5 and we can trigger the payback unconditionally after a while. This is just because I imagine it's easier to check for reasonable slippage when the order is created rather than when the order is old. Also, the user might want to know in advance we don't want to manually trigger the ETH payback.

I also wonder if it'd make sense to have a separate API path for creating ETH order. Me might also add extra info on order creation, for example a flag dontForcePayback that could be used in the future if a user doesn't want a high slippage and is willing to risk having to pay for getting the ETH out of the ETH-flow contract.

By the way, I propose to call the ETH-flow contract "handler" just for ease of typing (then instead of ETH-flow contract eip1271 order I could have written handler order). :grin:

[josojo]

Great, thanks for small session Federico. You helped me to understand that EIP1271 orders and eth-flow orders could be duplicates of each other. For that I added in Part 2:

Do not allow EIP1271 orders with the eth-flow contract as the owner. Otherwise, there might be duplication of orders.

Also, its completely right that the orders should be displayed in the batch explorer with their original origin and that we need to find a good way to translate between the two order types: Original order and ETH-flow order. I adapted above:

In part 2:

• Owner is the real user placing the order on the eth_handler
• Valid_to should be the real valid to specified by the user

such that all interface would show the correct order. And I adapted:

PART 4: Settling Eth-Flow Orders: While encoding the settlement of an order, following modifications need to be made:

• valid_to of the order should be set to uint(-1)
• owner should be the eth-flow contract
• (these two changes are the difference between the intended user order and the way order is placed by the eth flow contract, see table here )

Here, I also added the design choice trade off that we made:

Generally, the challenge of the proposal is that the there are two different kinds of orders - the real order with the real intend of the user and the order created by the eth-flow contract. There are small differences (cf. here) in these order types: The difference is so small that it does not justify the effort to introduce a completely new order type, but still big enough to introduce additional complexity. In the upper spec, the complexity is shifted into "payload encoding of an order". We could also go for the second approach and really introduce a new order type or at least a new field in the order that is representing the original user placing the eth-flow order.

[fleupold]

Can we maybe do a larger "design review" of this quite important feature and how it is best added into the current backend infrastructure? I think this task is a great start, I just want to make sure that before we start hacking we have input from all the right domain experts (likely with a synchronous discussion at the end) - @vkgnosis, @nlordell .

Generally, I wonder how much of the design really needs to be ETH-flow specific vs. how much can be built in a more generalizable way? E.g.

Parsing order placements from smart contract events:

We have been talking about adding support for this for a while (most recently year was considering building a smart contract that emits such events and they would need our infra to index those)

Create a new table ethflow_signatures_events

Why do they need a new table that seems very tailored to a specific feature? Aren't they structurally the same as smart contract orders and could just be stored in exactly the same way?

Generally I don't like the way of introducing a "fake" signature type that doesn't exist on the smart contract level. I'm worried that our offchain system will get messy with a superset of "real types" and require all kinds of "if this, then do that" logic.

What concretely is the extra information we need on a ETH flow order? Would other order types also benefit from this info? Do we want to adjust the DB schema for it or - if it's only for display purposes - can it live in the app_data?

[josojo]

Create a new table ethflow_signatures_events Why do they need a new table that seems very tailored to a specific feature? Aren't they structurally the same as smart contract orders and could just be stored in exactly the same way?

Currently, the presign table has the following structure:

CREATE TABLE presignature_events (
    block_number bigint NOT NULL,
    log_index bigint NOT NULL,
    owner bytea NOT NULL,
    order_uid bytea NOT NULL,
    signed boolean NOT NULL,
    PRIMARY KEY (block_number, log_index)
);

Fields like the signed variable is not needed and fields like valid_to are missing - this is important for an ETH-flow order, as the valid_to is different in the user_order and smart_contract order . Yes, I could add these fields to the presignature_events, but I thought it would be cleaner to have a clear separation with two tables instead of a one table with quite some null values. But yes, I am not sure about it.

Generally I don't like the way of introducing a "fake" signature type that doesn't exist on the smart contract level. I'm worried that our offchain system will get messy with a superset of "real types" and require all kinds of "if this, then do that" logic.

I agree that this is not optimal.

What concretely is the extra information we need on a ETH flow order? Would other order types also benefit from this info?

With the ethflow we only introduce the difference between users: real owner and eth-flow contract owner and the valid_to: real valid to and the uint(-1). I think we could also just extend the current database with the field: abstracted_owner and abstracted_valid_to. Then these fields can be used in other orders in the future as well.

--> Let me make another write-up for this approach together with a new order-type. In general, it might be more work, but it could be much cleaner.

Do we want to adjust the DB schema for it or - if it's only for display purposes - can it live in the app_data?

I think we do need this information in the DB. For example, if we want to ask for orders of the user, then this information should be quickly served from the api, hence, we would need to store this information somehow in the db to run queries. On the one hand side, we could start storing the ipfs medadata also in the backend and then this would work, but on the other side, I would rather not depend on slow ipfs to relay important information. The direct way through the API seems much more robust - even though it's "only for displaying".

[nlordell]

Generally, I wonder how much of the design really needs to be ETH-flow specific vs. how much can be built in a more generalizable way? E.g.

Unfortunately, I think we will always need some amount of ETH-flow specific logic (just because we want to be able to return ETH-flow orders for users).

In general, I do see the point of @fleupold's concerns around adding a new "fake" signature scheme to the database - it can get confusing and may cause some long term pains if we are not careful.

I think we could also just extend the current database with the field: abstracted_owner and abstracted_valid_to.

One thing that I don't quite understand, is why do we need a separate valid_to for the order and for ETH-flow contract? From what I understand, we pass in the order's valid_to with the signature bytes - so I'm not sure why it needs to be ∞.

Why do they need a new table that seems very tailored to a specific feature?

Won't this always be needed here though? We need a way to map an ETH-flow order to a user (which is different than the order signer - which is the ETH-flow contract). If we want special ETH-flow support in the API then I imagine we will need special logic and data to support this. In terms of making it more general - I am reluctant to pre-maturely abstract this without understanding what the common parts between ETH-flow orders and other potential on-chain order types are. For the SC that emits an Order event - I would imagine that this is just a presign order that won't need any special storage (and the order being emitted will have the same "actual" owner - which isn't the case for ETH-flow).

To me its unclear if we want owner in the orders table to be the signer address (i.e. the ETH-flow contract address) or the user owner.

abstracted_owner

I would call one the owner - which is the user that owns the order, and the other signer i.e. the address that is actually signing the signature (so the ETH-flow contract for ETH-flow orders, and == .owner in the general case).

[josojo]

One thing that I don't quite understand, is why do we need a separate valid_to for the order and for ETH-flow contract? From what I understand, we pass in the order's valid_to with the signature bytes - so I'm not sure why it needs to be ∞.

The reason is that the ETH-flow contract needs to be able to do refunds, in case the order was not settled. This means that the ETH-flow contract needs to know the executed amount of an order. This is stored in the settlement contract. If the order has the normal valid_to, then any solver could delete the "filled_amount" field of the order in the settlement contract after the valid_to has passed. And then the refunds would no longer work.

[nlordell]

Ah, yes of course! Thanks for clarifying.

[josojo]

Here is another try of the specification with more generalization + new datatype for orders

Specification:

• [ ] Part 1: Parsing order placements from smart contract events:

• We will use a generic parsing event for all smart contract orders: The events should have the following form:

  contract GPv2OnchainOrders {
  enum OnchainSigningScheme {
      Eip1271,
      PreSign,
  }
  
  struct OnchainSignature {
      OnchainSigningScheme scheme;
      bytes data;
  }
  
  event OrderPlacement(address indexed signer, GPv2Order.Data order, OnchainSignature signature, bytes data);
  event OrderCancellation(address indexed orderuid);
  // order updates are treated by a OrderCancelation event + OrderPlacement event.
  }

• ETH-Flow event parsing:

  • Add ABI for GPv2OnchainOrders contract and then use the existing EventUpdater to parse the new events
    • Event storing:
  • Create a new table on_chain_order_events with the fields: (block_number, log_index, signer, owner, valid_to, user_addres, ...)
    • Event maintenance with reorg protection:
  • Modify replace_events() to also consider the newest blocks from ethflowsignatures in last_event_block and then use replace_events() in order to enable the existing reorg_protection for event parsing also for the ethflowsignature events

• [ ] Part 2: Storing ETH-flow orders via API (While all eth-flow order information could be fully parsed from the events, we still require the front-end to place orders via the API, in order to allow the front-end to parse the quote information and check that its a recent quote)

  • Create a new version of the order tables with three new fields:

    • order_type:

      Enum OrderType {
      Standard,
      EthFlow, ...
      }

      Standard orders don't have a special treatment, they are just executed normally. The first addition would be eth-flow orders, but we could also in the future add: Waiting for Cow orders, or Liquidity orders.

    • signer: This will be the address originally placing on on-chain order. For normal orders, this field will be equal to the owner
    • meta_valid_to: This will be the valid_to from the original order

  • Adapt the placing order api interface to receive these three new fields. In order to make it non-breaking, all of the field would be optional and the OrderType would usually default to 'Standard'

  • Signature type: Ethflow orders will use the normal EIP1271 signature type

  • In validate_and_construct_order() create a separate if branch for the ethFlow order type that checks:

    • Quote is valid and recent
    • Owner is eth-flow contract
    • Signer is set
    • valid_to is uint(-1)
    • meta_valid_to should be the real valid to specified by the user
    • Receiver is not null and not the ethflow contract
    • Signature does not need to be checked, as it can always be constructed later on from the payload
    • Approvals are not checked, as they will be set from the EthFlow contract during deployment
    • The on-chain order does not yet have to be placed! Normally, we would expect that the API call for order placement happens first

  • Do not allow EIP1271 orders with the eth-flow contract as the owner. Otherwise, there might be duplication of orders.

• [ ] Part 3: Getting open orders for /auction endpoint:

  • Query only orders that we received via the API orders endpoint and for which we have a parsed ETH-flow event. This can be archived by appending two additional fields inORDERS_SELECT:

    ...,
    o.order_type = ‘ethftlow’ AND (
    CASE WHEN EXISTS 
        (
     SELECT *
     FROM onchain_signature_events p \
     WHERE o.uid = p.order_uid \
     AND o.signer = p.signer
     AND o.meta_valid_to = p.meta_valid_to
    LIMIT 1 ) \
    THEN 'true' \
    ELSE 'false' )AS eth_flow_order_pending \,

    And then we can use the new variables eth_flow_order_pending to determine whether an order was really placed on-chain and whether it should be considered.

  • Also the meta_valid_to condition needs to be checked.

• [ ] PART 4: Settling Eth-Flow Orders:

  • Create a new function: create_eth_flow_signature that creates the signature from order data.
  • Use the normal EIP1271 flow to settle the order

• [ ] Part 5: User payback service: – Periodically query all placed ETH orders that are not settled from the database

  • Check whether orders have been placed with sufficient slippage by comparing to quote
  • If all checks pass, initiate transactions that trigger the eth-flow contract to refund.

• [ ] Part 6: Displaying orders to the front-end – The function: get_user_orders needs to also consider signers, not just owners

• [ ] Part 7: Parse on-chain order invalidation:

  • Parse invalidation events from the blockchain using the EventUpdater
  • Put the events into a new database: invalidation_orders
  • To determine the status of an eth-flow order, always query invalidated_orders database as well.

---

After writing this, there are really two major questions:

• Do we want to generalize the on-chain order placement for future usecases?
• Do we want to generalize the concept of a "signer" = original owner and "meta_valid_to" for future usecases?
• Do we want to modify the fields in the order datatable or do we want to live with "fake" owners and valid_tos?

[josojo]

After the great input from our sync meeting, I am coming up with a new specification:

While we have not made final decision, I think we concluded for the upper questions:

Do we want to generalize the on-chain order placement for future usecases?

yes, as it might safe us quite some work in the future

Do we want to generalize the concept of a "signer" = original owner and "user_valid_to" for future usecases?

no, might be too custom

Do we want to modify the fields in the order datatable or do we want to live with "fake" owners and valid_tos?

Let's go for the cleaner version, although it much more work.

Concluded Vocabulary:

user: the user placing the eth-flow order
owner: the contract or address from whom the sell_tokens are withdrawn for an order.

valid_to: Normal valid to of an order, infinity for an eth-flow order
user_valid_to:  user intended valid to

Specification:

• [ ] Parsing order placements from smart contract events:

• We will use a generic parsing event for all smart contract orders: The exists a new contract GPv2OnchainOrders and it should be called from the ETH-flow contract:

  
  contract GPv2OnchainOrders {
  enum OnchainSigningScheme {
      Eip1271,
      PreSign,
  }
  
  struct OnchainSignature {
      OnchainSigningScheme scheme;
      bytes data;
  }
  
  event OrderPlacement(address indexed signer, GPv2Order.Data order, OnchainSignature signature,  bytes data);
  // order updates are treated by a OrderCancelation event + OrderPlacement event.
  
  function placeOrder(
      GPv2Order.Data memory order,
      OnchainSignature memory signature,
      bytes memory data
  ) public returns (bytes memory) {
      emit OrderPlacement(msg.sender, order, signature, quote,  data);
      return orderUid(order);
  }

The bytes data field has to contain the quote information, real user, the user_valid_to.
- Event parsing:
        - Add ABI for GPv2OnchainOrders contract and then use the existing `EventUpdater` to parse the new events
    - Event storing:
        - Create a new table `on_chain_order_events` with the fields:  (block_number, log_index,  signer, owner, order,  ...) 
    - Event maintenance with reorg protection:
        - We will only parse these events with a delay of X blocks, as it does not make sense to put "non-finalized orders" into the solver loop
       - (Or if we really want to, we can use `replace_events()`  in order to enable the existing reorg_protection for event parsing )
- [ ] Reserve quotes:
  - Create a new endpoint:

POST api/v1/reserve-quote { id: $QUOTE_ID, owner: $USER_FOR_THE_ORDER, // other stuff we may need }

users have to request a quote and then the quote needs to be used during order placement, Quotes will be valid for y minutes.
- [ ] Creating the EIP1271 order
     - During the creation of the `on_chain_order_events` entry for the newly parsed events, we need to run custom implementation for different signers. One of the custom implementation will be eth-flow implementation, it will be called if the `signer`= `eth-flow contract`. 
     - If orders are eth-flow orders, we decode the `bytes data` and receive the 'quote', `user` and `user_valid_to`:
     - The driver accepts the order, if:
       - The quote was reserved beforehand and is recent and valid
       - The sellamount is smaller than the sellamount specified in the quote (for sell orders)
     - Then we insert the new eth-flow orders into the orders database by building the EIP-1271 order with the owner = 'ethflow contract' and 'valid_to = infinity'
     - Also we insert into a new database `eth_flow_orders` with the fields( orderUId, user_valid_to, user) from the newly parsed data from the events.
     - (If the EIP1271 order is already exisiting, we would replace it with the new order - only the quote-information should be different anyhow)
- [ ] Getting open orders for /auction endpoint:
    - Here, we can query all orders as usual. Our newly created EIP1271 order representing the eth-flow will be included. In order to check for the order validity, we also have to query the user_valid_to from the `eth_flow_orders` and also check for invalidity of orders due to order-cancellations registered in the `invalidation_onchain_orders` table.

- [ ]  Settling Eth-Flow Orders:
    - Use the normal EIP1271 flow to settle the order

- [ ]  User payback service:
– Periodically query all placed ETH orders that are not settled from the database `eth_flow_orders`
    - Check whether orders have been placed with sufficient slippage by comparing to quote
    - If all checks pass, initiate transactions that trigger the eth-flow contract to refund.

- [ ] Displaying orders to the front-end
– The function: `get_user_orders` needs to query for a given user all entries from the `orders` datatable where user = order.owner and all entries from the 'eth_flow_orders' where `user = eth_flow_orders.user`. The api will serve additional field user and valid_to for eth_flow_orders and the front-end needs to adapt it. 
- The function: `get_order/{uid}` would have to look for the EIP1271 order and also check whether this order exists in the  
`eth_flow_orders` tables. If it does, then we need to also send back the `user_valid_to` and `user`

- [ ] Parse on-chain order invalidation:
 - Parse invalidation events from the blockchain using the `EventUpdater`
 - Put the events into a new database: `invalidation_onchain_orders`
 - The autopilot should only put orders into the next batch, if there are no invalidation available for the orderUIDs
 - Use the normal methods for dealing with reorgs in events.

------------
Challenges with this spec:
- paginated user order views:
  - In order to get the paginated view of the orders for a user, we would have to create a paginated view of eth_flow_orders and normal orders and then later merge these too. This should be possible. However, just getting a specific page will be quite hard - but its even hard right now.
- How many if else branches will we have with the new ordertype?
   - In this spec different parsing might be considered for different `signers` of the `OrderPlacement` event. Based on the signer, different orders could potentially be created in different tables. One of these tables is the new table: `eth_flow_orders`. Especially with each api call, we always have to check whether the orders requested are also `eth_flow_orders` and whether new information needs to be added. If we wanna avoid this overhead, we need to add the field  `user_valid_to` and `user` to the normal orders database. 
 This specification will not help with the `wait_for_cow orders`. 
- We have to send the 'user' and 'user_valid_to' onchain, which cost a little bit more than just sending it via the api.

I listed a lot of challenges. But I think it's the best solution! Wdyt?

[nlordell]

• We will only parse these events with a delay of X blocks, as it does not make sense to put "non-finalized orders" into the solver loop
• (Or if we really want to, we can use replace_events() in order to enable the existing reorg_protection for event parsing )

This is a very good point. I would slightly be inclined to use the first option because its simpler and avoids "bad batches". I think we can iterate on this detail in the future.

If orders are eth-flow orders, we decode the bytes data and receive the user and user_valid_to:

I think it also needs to include a "reservation code" to match the the reserved quote the order is being created with (i.e. something analogous to a quoteId).

[nlordell]

One thing I just thought about, is that we need some way to clean up "meta-expired" ETH-flow orders (so we don't have a bunch of "valid" orders that are actually expired and isValidSignature always returns false and we keep on checking it).

One solution is to have isValidSignature work for order cancellation as well, so we can run some maintenance that cancels ETH-flow orders?

This whole "meta-validity" is a complicated point for these orders.

[josojo]

I think it also needs to include a "reservation code" to match the the reserved quote the order is being created with (i.e. something analogous to a quoteId).

I put the reserved quote as apart of the event - second last field - as I think that it will be used in nearly all use-cases. The only down side is that we need to send zero-bytes, in case we use the concept "surplus from fee"

event OrderPlacement(address indexed signer, GPv2Order.Data order, OnchainSignature signature, bytes quote, bytes data);

[josojo]

This whole "meta-validity" is a complicated point for these orders.

This is really tricky. In the current spec, we have to join the 'user_validity' or 'meta_validity' to the normal order table in order to see the real validity. This might be quite some overhead on the databases. Maybe its easier to really put the 'user' and the 'user_validity' into the normal order database and then we don't have to do all these joints.

I think both solutions are fine. It's only a matter of what we prefer. Either a clean separation between core-protocol feature and adjacent protocol features, or easy-of-use and optimized requests. Probably, the clean separation will benefit us in the long-run, so I would prefer it.

[nlordell]

The only down side is that we need to send zero-bytes, in case we use the concept "surplus from fee"

For this, can't we just use the existing bytes data and have it be:

abi.encode(quoteReservation, otherData)

This way we keep a single "auxiliary data field" and we can remove the quoteReservation from it once we charge surplus from fees.

[nlordell]

Also, something that is not super clear to me is how useful this is:

event OrderCancellation(address indexed orderuid);

Wouldn't the order cancellation for pre-signature orders just be based on the PreSignature event and for EIP-1271 orders, couldn't it potentially happen off-chain (with a signature for example)?

For things that strictly require on-chain cancellation, can't we invalidate the order in the settlement contract directly?

[josojo]

Good catch nick! We have already the event OrderInvalidated in here:

 function invalidateOrder(bytes calldata orderUid) external {
        (, address owner, ) = orderUid.extractOrderUidParams();
        require(owner == msg.sender, "GPv2: caller does not own order");
        filledAmount[orderUid] = uint256(-1);
        emit OrderInvalidated(owner, orderUid);
    }

I will take it out

[fleupold]

Really nice iteration, I like the new proposals a lot!

Two thoughts (but feel free to ignore):

If we wanna avoid this overhead, we need to add the field user_valid_to and user to the normal orders database.

A third option could be to have all core data for all orders in one table, and then a pointer to a separate "meta_data_table" (can be different for eth_flow vs wait_for_cow_orders). This is closer to the first revised spec, but would still preserve database third normal form [1].

This whole "meta-validity" is a complicated point for these orders.

This all comes from the possibility of clearing storage from expired orders in the settlement contract, which is actually no longer profitable from a gas perspective. I personally think it would also be an option to forbid clearing this storage as part of the "rules of the game" and have the payback service check that no such clearance has happened (in which case we would alert, slash the violating solver and figure out manually if the user should be refunded or not). It's a less trustless solution, but if it otherwise adds a lot of complexity to the codebase (for a feature which if we ever redo the settlement contract we would get rid off) it could be an option.

[nlordell]

A third option could be to have all core data for all orders in one table, and then a pointer to a separate

One issue I have with this is that it implies that the "core protocol orders table" has some knowledge of orders "built on top" of the protocol. I would invert this a bit and say that the separate "meta_data_table" (so eth_flow_orders and wait_for_cow_orders in your example) should have pointers to the core table (they can use the well-defined order UIDs for this). This way, if you imagine a decentralized future with a distributed orderbook, we can still offer ETH-flow on CowSwap without it being part of the distributed "core order data storage".

[josojo]

This whole "meta-validity" is a complicated point for these orders. This all comes from the possibility of clearing storage from expired orders in the settlement contract, which is actually no longer profitable from a gas perspective. I personally think it would also be an option to forbid clearing this storage as part of the "rules of the game" and have the payback service check that no such clearance has happened (in which case we would alert, slash the violating solver and figure out manually if the user should be refunded or not). It's a less trustless solution, but if it otherwise adds a lot of complexity to the codebase (for a feature which if we ever redo the settlement contract we would get rid off) it could be an option.

@fleupold how serious are we about the idea: one settelment contract per market maker? If we wanna advocate in this direction, then we have to approve new settlement contracts anyways to the allowance manager - (and redeploy the allowance manger). Then we could also approve a new settlement contract for the eth-flow orders that don't have free_storage functionality. If we say that this is our long-term fix, then I could live with a short-term "broken security" for eth-flow orders. Otherwise, I find it a tough compromise, as the value of an eth-flow order could be much more than the currenntly existing security bonds for solvers.

[josojo]

Here is yet another iteration without the "quote reservation endpoint". Nick and I concluded that it would not necessarily be needed.

Concluded Vocabulary:

user: the user placing the eth-flow order
owner: the contract or address from whom the sell_tokens are withdrawn for an order.

valid_to: Normal valid to of an order, infinity for an eth-flow order
user_valid_to:  user intended valid to

Specification:

• [ ] Parsing order placements from smart contract events:

• We will use a generic parsing event for all smart contract orders: The exists a new contract GPv2OnchainOrders and it should be called from the ETH-flow contract:

  
  contract GPv2OnchainOrders {
  enum OnchainSigningScheme {
      Eip1271,
      PreSign,
  }
  
  struct OnchainSignature {
      OnchainSigningScheme scheme;
      bytes data;
  }
  
  event OrderPlacement(address indexed signer, GPv2Order.Data order, OnchainSignature signature,  bytes data);
  // order updates are treated by a OrderCancelation event + OrderPlacement event.
  
  function placeOrder(
      GPv2Order.Data memory order,
      OnchainSignature memory signature,
      bytes memory data
  ) public {
      emit OrderPlacement(msg.sender, order, signature, quote,  data);
  }

The bytes data field has to contain the quote information, real user, the user_valid_to.
- Event parsing:
        - Add ABI for GPv2OnchainOrders contract and then use the existing `EventUpdater` to parse the new events
    - Event storing:
        - Create a new table `on_chain_order_events` with the fields:  (block_number, log_index,  signer, owner, order,  ...) 
    - Event maintenance with reorg protection:
        - We will only parse these events with a delay of X blocks, as it does not make sense to put "non-finalized orders" into the solver loop
- [ ] Extend quotes validity:
 - Store quotes longer in the database depend on the signatures:
  -  EIP1271 quotes should be stored 10 mins: If orders are placed via on-chain events, then it might take up to 10 minutes to mine a transaction.
  - PreSign quotes should be stored up to 24 hours: If orders are placed via the gnosis-safe (multisig) with an on-chain process, then it might take even longer to organize all the signatures and hence the validity must be even longer.
  - All other quotes should be continued to be stored for only a short time.

- [ ] Creating the EIP1271 order
     - During the creation of the `on_chain_order_events` entry for the newly parsed events, we need to run custom implementation for different signers. One of the custom implementations will be eth-flow implementation, it will be called if the `signer`= `eth-flow contract`. 
     - If orders are eth-flow orders, we decode the `bytes data` and receive the 'quoteID', `user` and `user_valid_to`:
     - The driver accepts the order, if:
       - The quote was reserved beforehand and is recent and valid
       - The sellAmount is smaller than the sellAmount specified in the quote (for sell orders)
     - Then we insert the new eth-flow orders into the orders database by building the EIP-1271 order with the owner = 'ethflow contract' and 'valid_to = infinity'
     - Also we insert into a new database `eth_flow_orders` with the fields( orderUId, user_valid_to, user) from the newly parsed data from the events.
     - (If the EIP1271 order is already exisiting, we would replace it with the new order - only the quote-information should be different anyhow)
- [ ] Getting open orders for /auction endpoint:
    - Here, we can query all orders as usual. Our newly created EIP1271 order representing the eth-flow will be included. In order to check for the order validity, we also have to query the user_valid_to from the `eth_flow_orders` and also check for invalidity of orders due to order-cancellations registered in the `invalidation_onchain_orders` table.

- [ ]  Settling Eth-Flow Orders:
    - Use the normal EIP1271 flow to settle the order

- [ ]  User payback service:
– Periodically query all placed ETH orders that are not settled from the database `eth_flow_orders`
    - Check whether orders have been placed with sufficient slippage by comparing to quote
    - If all checks pass, initiate transactions that trigger the eth-flow contract to refund.

- [ ] Displaying orders to the front-end
– The function: `get_user_orders` needs to query for a given user all entries from the `orders` datatable where user = order.owner and all entries from the 'eth_flow_orders' where `user = eth_flow_orders.user`. The api will serve additional field user and valid_to for eth_flow_orders and the front-end needs to adapt it. 
- The function: `get_order/{uid}` would have to look for the EIP1271 order and also check whether this order exists in the  
`eth_flow_orders` tables. If it does, then we need to also send back the `user_valid_to` and `user`

- [ ] Parse on-chain order invalidation:
 - Parse invalidation events from the blockchain using the `EventUpdater`
 - Put the events into a new database: `invalidation_onchain_orders`
 - The autopilot should only put orders into the next batch, if there are no invalidation available for the orderUIDs
 - Use the normal methods for dealing with reorgs in events.

------------
Challenges with this spec:
- paginated user order views:
  - In order to get the paginated view of the orders for a user, we would have to create a paginated view of eth_flow_orders and normal orders and then later merge these too. This should be possible. However, just getting a specific page will be quite hard - but its even hard right now.
- How many if else branches will we have with the new ordertype?
   - In this spec different parsing might be considered for different `signers` of the `OrderPlacement` event. Based on the signer, different orders could potentially be created in different tables. One of these tables is the new table: `eth_flow_orders`. Especially with each api call, we always have to check whether the orders requested are also `eth_flow_orders` and whether new information needs to be added. If we wanna avoid this overhead, we need to add the field  `user_valid_to` and `user` to the normal orders database. 
 This specification will not help with the `wait_for_cow orders`. 
- We have to send the 'user' and 'user_valid_to' onchain, which cost a little bit more than just sending it via the api.

[fleupold]

@fleupold how serious are we about the idea: one settelment contract per market maker?

I think we are serious about it, but it's still a bit early to say how likely we end up going down that route. First, we need to get market makers on board and then see how much of a pain point the current bonds are (or if there are other ways around reducing them). I think there are a few challenges involved when it comes to accounting (as well as rounding errors in the solver) so I'm a bit reluctant to say "let's try this model out with the ETH flow" to get some additional insight into the feasibility of this approach. Wdyt?

[josojo]

I'm a bit reluctant to say "let's try this model out with the ETH flow" to get some additional insight into the feasibility of this approach.

I am also not fond of the idea. Because, to my knowledge, it requires a redeployment of the GPv2VaultRelayer contract, which is a huge overhead from a technical and legal perspective.

Otherwise, I find it a tough compromise, as the value of an eth-flow order could be much more than the currently existing security bonds for solvers.

Do you actually agree with this assessment? Otherwise, if you want us to use not use the user_valid_to and normal valid_to, then this will be significant change and we should do it now.