Multi collateral support - add concurrent liquidation tests #8422
Conversation
iomekam
force-pushed
the
8270-concurrent-liquidation
branch
2 times, most recently
from
6486d53 to
41bcf85
Compare
iomekam
changed the title
iomekam
force-pushed
the
8270-concurrent-liquidation
branch
from
41bcf85 to
9af1e08
Compare
| t.log('building proposal'); | ||
| const proposal = await buildProposal({ | ||
| package: 'builders', | ||
| packageScriptName: 'build:add-STARS-proposal', |
There was a problem hiding this comment.
Out of scope, but we should parameterize this function to addNewCollateral and take the collateral name. @Chris-Hibbert needed (and made) such a thing in https://github.com/Agoric/agoric-sdk/pull/8398/files#diff-3c4081d1865d2d24a7abaee93a075a8852065c32783fbe25265746c13441875bR38-R45
|
|
||
| const test = anyTest as TestFn<LiquidationTestContext>; | ||
|
|
||
| const atomSetup = { |
There was a problem hiding this comment.
I'm not reviewing the arithmetic. I assume you'll sync with PM on the correctness per their spec
packages/boot/test/bootstrapTests/test-liquidation-concurrent-1.ts
Outdated
Show resolved
Hide resolved
packages/boot/test/bootstrapTests/test-liquidation-concurrent-1.ts
Outdated
Show resolved
Hide resolved
packages/boot/test/bootstrapTests/test-liquidation-concurrent-1.ts
Outdated
Show resolved
Hide resolved
| const buyer = await walletFactoryDriver.provideSmartWallet('agoric1buyer'); | ||
| const minter = await walletFactoryDriver.provideSmartWallet('agoric1minter'); | ||
|
|
||
| const setupVault = async ( |
There was a problem hiding this comment.
isn't this the same as in concurrent-1? please dedupe
| return t.context.shutdown && t.context.shutdown(); | ||
| }); | ||
|
|
||
| const checkFlow2b = async (t: ExecutionContext<LiquidationTestContext>) => { |
There was a problem hiding this comment.
I asked socratically :) (that 2b is for a different test flow than this one)
There was a problem hiding this comment.
I asked socratically :) (that 2b is for a different test flow than this one)
| }; | ||
|
|
||
| // Reference: Flow 2b from https://github.com/Agoric/agoric-sdk/issues/7123 | ||
| test.serial('scenario: Flow 2b', async t => { |
There was a problem hiding this comment.
the helper function factoring is for re-use in multiple tests. since there's only in in this case it's an unwarranted complication
iomekam
force-pushed
the
8270-concurrent-liquidation
branch
from
9af1e08 to
3d149fe
Compare
iomekam
force-pushed
the
8270-concurrent-liquidation
branch
from
94694b6 to
de35fd0
Compare
iomekam
force-pushed
the
8270-concurrent-liquidation
branch
from
de35fd0 to
90b2cc0
Compare
| collateralBrandKey: string, | ||
| t: ExecutionContext<LiquidationTestContext>, | ||
| ) => { | ||
| // TODO: we'd like to have this work on any brand |
There was a problem hiding this comment.
nice solution! creates the right abstraction and we can improve its robustness later
| for (const { collateralBrandKey } of cases) { | ||
| await ensureVaultCollateral(collateralBrandKey, t) | ||
| } |
There was a problem hiding this comment.
this lint error could be ignored for tests but it still would be better to:
await Promise.all(
cases.map(({collateralBrandKey}) => ensureVaultCollateral(collateralBrandKey, t));
);| { | ||
| // --------------- |
There was a problem hiding this comment.
consider restoring this block to reduce churn for indentation.
iomekam
force-pushed
the
8270-concurrent-liquidation
branch
from
90b2cc0 to
c59e114
Compare
iomekam
force-pushed
the
8270-concurrent-liquidation
branch
from
c59e114 to
c900417
Compare
closes: #8270
Description
Adds test that validate concurrent liquidation of multiple collaterals.
Security Considerations
N/A
Scaling Considerations
N/A
Documentation Considerations
N/A
Testing Considerations
Adding new test, that have been validated locally
Upgrade Considerations
N/A