test: fast-usdc contract flow #10572
Conversation
Deploying agoric-sdk with
|
| Latest commit: |
dcc6feb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://54713ebe.agoric-sdk.pages.dev |
| Branch Preview URL: | https://dc-fc-contract-tests.agoric-sdk.pages.dev |
|
currently (e049c85) passing tests: |
dckc
force-pushed
the
dc-fc-contract-tests
branch
2 times, most recently
from
e4052ae to
e049c85
Compare
|
@0xpatrickdev , @turadg , based on today's discussion of scope, I think this is ready for review. |
|
Why is this inter protocol test failing??? strangely, it reproduces locally. |
There was a problem hiding this comment.
LGTM. Only have one comment that really needs addressing.
I am not the craziest about the single shared test context and test.serials. It makes it so that we can't do yarn test ... -m 'STORY01:*' to debug or work on an individual test. Speaking offline it seemed more cost effective to structure this way, so I won't block on this.
| */ | ||
| onRejected(_result, _ctx) { | ||
| onRejected(reason, ctx) { | ||
| trace('transfer rejected!', reason, ctx); |
There was a problem hiding this comment.
should this have a
| @@ -98,7 +98,7 @@ export const connectionKey = (chainId1, chainId2) => { | |||
| */ | |||
| const reverseConnInfo = connInfo => { | |||
| const { transferChannel } = connInfo; | |||
| return { | |||
| return harden({ | |||
There was a problem hiding this comment.
I hit this as well: 33f26e9
|
|
||
| console.log('chainHub: registering assets', Object.keys(assetInfo || {})); |
| const assetInfo = harden( | ||
| Object.fromEntries([ | ||
| assetOn('uusdc', 'noble'), | ||
| [uusdcOnAgoric, agUSDCDetail], |
There was a problem hiding this comment.
assetOn is nifty. Why not also use it here?
There was a problem hiding this comment.
It was used to define these 2.
uusdOnAgoric is exported for use elsewhere.
| @@ -264,13 +275,13 @@ export const prepareSettler = ( | |||
| * @param {SettlerTransferCtx} ctx | |||
| * | |||
| * @typedef {{ | |||
| * txHash: EvmHash; | |||
| * txHash: EvmHash | undefined; | |||
There was a problem hiding this comment.
When can this be undefined?
There was a problem hiding this comment.
Only in the case that we're not handling correctly (#10577). I might as well fix it now.
| }); | ||
|
|
||
| test.todo( | ||
| 'PERF: Target: settlement completes in a few minutes (after USDC is minted)', |
There was a problem hiding this comment.
I don't think this will be the right place for this, since much of the latency is dependent on blocks and bridges, so maybe we should omit this todo. Would we do this in multichain-testing? a testnet?
There was a problem hiding this comment.
I don't think this will be the right place for this,
I agree, but this is where I was taking notes on product requirements. I'll take this one out, but note that there are also TODOs here that should be satisfied by exo tests. When the TODOs get done, we can prune these notes.
since much of the latency is dependent on blocks and bridges, so maybe we should omit this todo. Would we do this in multichain-testing? a testnet?
We have plans for bootstrap tests to measure computrons as well as multichain-testing and testnets.
| // TODO/WIP: cust.racer3.checkSent(t, since(bridgePos), 'forward - LP depleted'); | ||
| await transmitTransferAck(); | ||
| // Nothing we can check here, unless we want to inspect calls to `trace`. | ||
| // `test/exos/advancer.test.ts` covers calls to `log: LogFn` with mocks. | ||
| // This is still helpful to call, so we can observe "Advance transfer | ||
| // fulfilled" in the test output. | ||
| await transmitTransferAck(); | ||
| await transmitTransferAck(); | ||
| await Promise.all([mint(sent1), mint(sent2), mint(sent3)]); |
There was a problem hiding this comment.
This is cool, but I'm struggling to understand how we plan to check the results of the Promise.all. I don't think we'll have any bridge messages that indicate insufficient funds (advancer's log would know though.) Will we wait for txs to be posted to vstorage?
Regardless, consider specifying in the TODO/WIP
There was a problem hiding this comment.
Yes, as I was working on this test, I realized that our mock bridge stuff only works one tx at a time. I'm not inclined to go on at any length about what's TODO/WIP here just now.
| const bridgeTraffic = since(bridgePos); | ||
| await mint(sent); | ||
| custEmpty.checkSent(t, bridgeTraffic, 'forward'); | ||
| t.log('No advancement, just settlement'); |
There was a problem hiding this comment.
Consider a TODO somewhere here to ensure Observed and/or Forwarded statuses are recorded in vstorage
|
|
||
| // advancedEarly stuff | ||
| test.todo( | ||
| 'C12 - Contract MUST only pay back the Pool only if they started the advance before USDC is minted', |
There was a problem hiding this comment.
This is advance early, but also could be the same scenario about. We could inspect the outgoing LCA bridge messages to confirm they sent the requested amount and not the net of fees amount
There was a problem hiding this comment.
We could inspect the outgoing LCA bridge messages to confirm they sent the requested amount and not the net of fees amount
checkSent with 'forward' does check that.
But we didn't test the advancedEarly case, i.e. the case where the mint comes in between ADVANCING and ADVANCED.
|
|
||
| test.todo('C18 - forward - MUST log and alert these incidents'); | ||
|
|
||
| test.serial.failing( |
There was a problem hiding this comment.
As I say, might as well fix it now.
non-hardened objects can't go in stores such as `connectionInfos`.
- realistic USDC denom in test supports
10 tests passed 1 known failure 2 tests skipped 7 tests todo Done in 3.43s. - [skip] LP borrow - TODO: move to exo test - [skip] LP repay - TODO: move to exo test - [todo] PERF: Target: settlement completes in a few minutes (after USDC is minted) - [todo] C21 - Contract MUST log / timestamp each step in the transaction flow - [todo] document metrics storage schema - [todo] get metrics from vstorage - [todo] C12 - Contract MUST only pay back the Pool only if they started the advance before USDC is minted - [todo] C18 - forward - MUST log and alert these incidents - [todo] fee levels MUST be visible to external parties - i.e., written to public storage - known failure: unknown transaction settler should Leave funds in SettlementAccount. - use oracle, customer in advance happy path - factor out makeOracleOperator - registerAsset() from the test jig doesn't work: brands in the contract are in a different "address space" - use realistic USDC denom - shared context for contract tests - customer sends before LPs deposit - racer tests - flatten makeLP - don't mix with pourPayment - derive publicFacet etc. from instance as client would - skip LP test that used test-only methods - prune outdated Oracle rights test
dckc
force-pushed
the
dc-fc-contract-tests
branch
from
ee85747 to
2f4bafe
Compare
dckc
force-pushed
the
dc-fc-contract-tests
branch
2 times, most recently
from
17b6d85 to
dcc6feb
Compare
refs: #10388
closes: #10577
doesn't close until
Description / Testing Considerations
postponed:
some
test.todo()s are out of scope of contract flow tests. Some are more feasible as exo tests:an some are after M1:
Security / Scaling / Upgrade Considerations
n/a
Documentation Considerations
Internal design docs are assumed background knowledge.