Conversation
0xpatrickdev
force-pushed
the
pc/fusdc-vstorage-updates
branch
from
4909feb to
e3d94c3
Compare
Deploying agoric-sdk with
|
| Latest commit: |
475e720
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2706c7b1.agoric-sdk.pages.dev |
| Branch Preview URL: | https://pc-fusdc-vstorage-updates.agoric-sdk.pages.dev |
turadg
left a comment
turadg
left a comment
There was a problem hiding this comment.
Requested a recorderkit
Also some tests should be affected by these changes. I just realized the tests I wrote in https://github.com/Agoric/agoric-sdk/pull/10633/commits haven't landed. I'll trim that PR to get them in
| E(settlementAccount).getAddress(), | ||
| ]), | ||
| ); | ||
| await publishAddresses(storageNode, { |
There was a problem hiding this comment.
publishAddresses calling a different publishAddresses is quite confusing. I first thought this should be inlined but I imagine you wanted some documentation and type annotation of the helper.
Usually we would get that with a RecorderKit. Then I thought it's not necessary in this contract because the "on-chain can read same values as off-chain" requirement isn't pertinent to this contract that only works off-chain.
But thinking about it more, it's imaginable that another contract would want a reference to these accounts. E.g. to automate pool participation.
So I think an addressRecorderKit.recorder.write should be used to write these values. A makeRecorderKit is already available in scope.
There was a problem hiding this comment.
Good feedback, contracts wanting to consume this value crossed my mind over the weekend but I forgot about it when revisiting the changes today.
As an alternative to a recorderKit, what do you think about a getAddresses public facet method? This might be more fitting since we expect the values to be static, or at least append only?
There was a problem hiding this comment.
Good point. Since it never changes a RecorderKit is overkill. To emphasize the static nature consider getStaticInfo() or something like that where we could include all static info about the contract
dckc
force-pushed
the
dc-start-fast-usdc
branch
from
8874419 to
c9ba09e
Compare
0xpatrickdev
force-pushed
the
pc/fusdc-vstorage-updates
branch
from
e3d94c3 to
bf7b154
Compare
0xpatrickdev
force-pushed
the
pc/fusdc-vstorage-updates
branch
2 times, most recently
from
21d5049 to
287e5be
Compare
| }); | ||
| }, | ||
| async publishAddresses() { | ||
| !baggage.has(ADDRESSES_BAGGAGE_KEY) || Fail`Addresses already published`; |
- include `poolMetrics` and account addresses
- ensure contract consumers have access to the contract's orch account addresses which are currently only published to vstorage. since these are not expected to change, a public facet method seems more suitable than a recorderKit
0xpatrickdev
force-pushed
the
pc/fusdc-vstorage-updates
branch
from
287e5be to
475e720
Compare
2 participants
incidental FUSDC changes
Description
PoolMetricsto child nodegetStaticInfomethod to PublicFacet to return orch account addressesSecurity Considerations
None
Scaling Considerations
None
Documentation Considerations
None
Testing Considerations
Adds snapshot tests for vstorage and demonstrates usage of
getStaticInfoin unit test.Upgrade Considerations
None, unreleased contract