Create DummyCollateralizedContract for test
#24
Conversation
| return true; | ||
| } | ||
|
|
||
| function getExpectedRepaymentValue( |
There was a problem hiding this comment.
You will want to add some sort of ability to programmatically change the returned expectedRepaymentValue and valueRepaid -- these values affect logic of whether a collateralized asset can be seized or not, and you'll want to test for that.
There was a problem hiding this comment.
Yup. This is just scaffolding. That functionality will come in a future PR.
| // the lockup period must occur at some point in the future. | ||
| require(lockupPeriodEndTimestamp > block.timestamp); | ||
|
|
||
| // the agreement cannot be collateralized more than once. | ||
| require(collateralForAgreementID[agreementID].lockupPeriod == 0); |
There was a problem hiding this comment.
Let's explicitly document the assumption we're making here -- i.e. that lockupPeriod will never be 0 for for an agreement that has already been collateralized
| return true; | ||
| } | ||
|
|
||
| function getExpectedRepaymentValue( |
| mockRegistry.address | ||
| ); | ||
|
|
||
| const collateralContractWeb3Contract = |
There was a problem hiding this comment.
Let's add a comment here similar to that found in charta/test/ts/integration/debt_kernel.ts about why we use this wonky means of instantiating a new contract. I'm creating a pivotal task for us to add a deploy method to our generated typings.
|
|
||
| // TransferProxy is granted an allowance of 3 ether from PAYER. | ||
| await mockToken.mockAllowanceFor.sendTransactionAsync( | ||
| PAYER, collateralContract.address, Units.ether(5), |
There was a problem hiding this comment.
3 ether or 5 ether?
| )).to.eventually.be.rejectedWith(REVERT_ERROR); | ||
| }); | ||
|
|
||
| it("should throw if the collateral fails to transfer", async () => { |
There was a problem hiding this comment.
For all 4 of the above invariants, I think it would be better if we changed the collateral contract to "throw" errors using the LogError pattern we use in DebtKernel -- given that Solidity doesn't (yet) support throwing with a reason string. This makes it easier to output human readable errors in UIs
There was a problem hiding this comment.
Also -- we should add one more invariant to the Collateralized contract: outputting a LogError if allowance to the contract is not sufficient to execute transferFrom
…row once collateral is withdrawn
saturnial
force-pushed
the
collateral-tests
branch
from
125c5dd to
0702923
Compare
This reverts commit 47b6c32.
Changes requested during review of Collateral unit tests
Unit test suite for the `Collateralized` abstract interface
No description provided.