Skip to content
This repository was archived by the owner on Jan 18, 2023. It is now read-only.

Conversation

asoong
Copy link
Contributor

@asoong asoong commented Jul 2, 2018

  • Updated structs, removing overall order data header
  • Remove previously implemented MockExchangeOrderHandler which was added just to test the ExchangeOrderHandler library
  • Parse header as part of fillOrder
  • Add ExchangeDispatcher as an Core extension to manage valid exchanges that can be used to fill the issuanceOrder

@asoong asoong requested a review from a team July 2, 2018 07:43
);
require(
to < b.length,
to <= b.length,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@felix2feng did they respond to your question about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope

const exchange = _.sample(EXCHANGES);
exchangeOrderDatum.push(paddedBufferForData(exchange));

const orderLength = _.random(120, 160)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hoping to sub any kind of util function we write later that generates a real exchange order that mirrors what setProtocol.js will return

Copy link
Contributor

Choose a reason for hiding this comment

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

I've been already starting to generate more realistic orders. We can fix after my PR goes through

);
require(
to < b.length,
to <= b.length,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nope

const exchange = _.sample(EXCHANGES);
exchangeOrderDatum.push(paddedBufferForData(exchange));

const orderLength = _.random(120, 160)
Copy link
Contributor

Choose a reason for hiding this comment

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

I've been already starting to generate more realistic orders. We can fix after my PR goes through

import { CoreState } from "../lib/CoreState.sol";


// /**
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are there 2 types of comments here?

/* ============ Structs ============ */

struct State {
// Mapping of exchagne enumeration to address
Copy link
Contributor

Choose a reason for hiding this comment

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

"exchange"

// ============ Structs ============

struct OrderHeader {
uint8 exchange;
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI - Currently, we don't get any storage savings from packing with just a uint8

* Adding lib bytes and removing coverage of it.

* Add slice

* Add content address
@coveralls
Copy link

coveralls commented Jul 3, 2018

Pull Request Test Coverage Report for Build 384

  • 16 of 16 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 381: 0.0%
Covered Lines: 182
Relevant Lines: 182

💛 - Coveralls

@asoong asoong force-pushed the alex/decode_order_data branch 2 times, most recently from e35fc9b to f240377 Compare July 3, 2018 03:39
@asoong asoong force-pushed the alex/decode_order_data branch from f240377 to 5e0f168 Compare July 3, 2018 03:42
@asoong asoong merged commit 2010a1f into master Jul 3, 2018
@asoong asoong deleted the alex/decode_order_data branch July 3, 2018 04:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants