4489 walletFactory virtual #6165
Conversation
turadg
force-pushed
the
4489-walletFactory-virtual
branch
from
e931e72
to
0a6cc0b
Compare
| @@ -153,7 +154,8 @@ test('want stable', async t => { | |||
| t.is(purseBalance(computedState, stableBrand), swapSize - 1n); | |||
| }); | |||
|
|
|||
| test('govern offerFilter', async t => { | |||
| // FIXME this broke after https://github.com/Agoric/agoric-sdk/pull/6166 | |||
There was a problem hiding this comment.
please create an issue and assign to me. (Unless you want to fix it in this PR.)
| brandDescriptors: makeScalarMapStore(), | ||
| // To ensure every offer ID is unique we require that each is a number greater | ||
| // than has ever been used. This high water mark is sufficient to track that. | ||
| lastOfferId: 0, |
There was a problem hiding this comment.
does lastOfferId need to survive upgrade?
There was a problem hiding this comment.
it's a SHOULD not a MUST. if it doesn't survive it's easy to determine from the offerStatus update in RPC. I'll make a note of that.
| getDepositFacet: M.call().returns(M.eref(M.any())), | ||
| getOffersFacet: M.call().returns(M.eref(M.any())), |
There was a problem hiding this comment.
These facets should be simple to describe.
There was a problem hiding this comment.
what's the value? these are input guards. why runtime validate the return?
seems like busy work, especially when we have to write types twice (until #6160)
There was a problem hiding this comment.
I'm worried that we'll do something like #6160, but have incomplete types on the returns because they're currently not enforced, so they'll all say .returns(M.promise()) or .returns(M.eref(M.any()).
Not suggesting a change here, just registering my concern.
turadg
force-pushed
the
4489-walletFactory-virtual
branch
from
0a6cc0b
to
79f3df2
Compare
turadg
added
the
automerge:no-update
There was a problem hiding this comment.
I approve on the basis that the conversion to heapFarInstances looks clean.
I haven't looked at the smartWallet design enough to have confidence that the local rearrangements haven't done violence to it. Is there anyone who understands wallet/smartWallet well enough to review?
turadg
force-pushed
the
4489-walletFactory-virtual
branch
from
79f3df2
to
47be3cc
Compare
|
79f3df2 passed deployment test and the only change since then is fixing a TS lint error so I'm adding bypass:integration to get this in faster. |
turadg
force-pushed
the
4489-walletFactory-virtual
branch
from
47be3cc
to
659ad58
Compare
closes: #4489
Description
Updates the smart wallet contract to be virtualized.
This is my first use of the new Far Class stuff. I cut some paper cuts and tried to sand the edges with refactor(store): types for interface-tools but stopped eventually. Many types in this PR have regressed to
anybut addressing that can wait to after launch.Security Considerations
More guards
Documentation Considerations
--
Testing Considerations
There's one test that started failing after I rebased, I think due to #6166
"govern offerFilter"test