Brian+alex/integrate taker wallet #119
Conversation
| await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); | ||
| }); | ||
|
|
||
| describe.only("#fillOrder", async () => { |
bweick
force-pushed
the
brian+alex/integrate_taker_wallet
branch
from
f38411c to
498a348
Compare
Pull Request Test Coverage Report for Build 702
💛 - Coveralls |
| ); | ||
|
|
||
| // Record taker token and amount to return values | ||
| uint256 orderCount = scannedBytes >> 6; |
There was a problem hiding this comment.
How does this work? Does it increment one at a time?
There was a problem hiding this comment.
Yes, but this is particular to our workflow because we scan 64 bytes at at time, so you can think of it as counting from 0 starting on the 7th bit.
| ); | ||
|
|
||
| // Transfer component tokens from wrapper to vault | ||
| batchDepositInternal( |
There was a problem hiding this comment.
Wow that's really nice!
| } | ||
|
|
||
| settleAccounts(_order, _fillQuantity, requiredMakerTokenAmount, makerTokenAmountUsed); | ||
| settleAccounts( |
There was a problem hiding this comment.
Do we have the logic to send remaining makerTokens to the taker/arb yet?
There was a problem hiding this comment.
That's in the settle accounts function itself
bweick
force-pushed
the
brian+alex/integrate_taker_wallet
branch
from
891cb9a to
6e0e7e2
Compare
…ved some IssuanceOrder logic not relevant to taker wallets
bweick
force-pushed
the
brian+alex/integrate_taker_wallet
branch
from
7101d90 to
34c4175
Compare
| address _tokenAddress, | ||
| uint _quantity | ||
| ) | ||
| public |
There was a problem hiding this comment.
shouldn't this be internal?
There was a problem hiding this comment.
Yes...important catch. Imma push a fix immediately.
| * The ICoreIssuance Contract defines all the functions exposed in the CoreIssuance | ||
| * extension. | ||
| */ | ||
| contract ICoreAccounting { |
There was a problem hiding this comment.
It's a little weird to be using an abstract class here instead of interface (same for ICoreIssuance)...I think solidity throws warnings if methods in interfaces aren't external which wouldn't work here though but I wonder if we should be splitting up the naming of interfaces from abstract classes
There was a problem hiding this comment.
Yeah we can't use internal functions in libraries so we have to define it as a contract. Which unfortunately makes us have to be more careful with inheritance when the Core inherits the IssuanceOrder extension lest we overwrite the actual logic of the batchDepositInternal function. If we want to name it something else we can do that since it isn't technically an interface thought it is intended to act as such.
5 participants
No description provided.