feat(fast-usdc): write chain policies to vstorage

[samsiegart]

closes #10512

Description

Adds the shape/type for the chain policies, and makes the fast-usdc.start script write them to vstorage

Security Considerations

Only the start script and the contract have permission to write to the vstorage node

Scaling Considerations

This is a single invocation that runs once per core-eval, so shouldn't impact performance over time.

Documentation Considerations

There's an example policy in the test snapshot

Testing Considerations

Wrote a bootstrap test that reads vstorage to verify the chain policies are published.

Upgrade Considerations

The core eval would have to write to vstorage again to update the policies

[dckc]

I think there's a norm that stuff that goes to vstorage is also available to other on-chain consumers via getPublicTopics(). IOU a link

But I'm not sure in this case. AFAICT, the contract makes no use of the policy whatsoever. So maybe a 1-time write from the .start.js core-eval code is the way to go.

maybe @turadg or @0xpatrickdev knows more about the design?

[dckc]

This gives the feed access to more of vstorage than it needs.
Please move the makeChildNode call up into the contract start. Maybe makeRecorderKit too. The feed only needs the recorder, it looks like.

Things may need to move around a bit due to...

agoric-sdk/packages/fast-usdc/src/fast-usdc.contract.js

Line 190 in ae29bc2

// ^^^ Define all kinds above this line. Keep remote calls below. vvv

The prepareFeedKit(...) has to stay above the line but makeFeedKit(...) can be below it.

[dckc]

I think there's a norm that anything the contract writes to vstorage is also available via the public facet via getPublicTopics. IOU a pointer

That would require a persistent identity for the subscriber, so the recorderKit would have to be stored durably using makeOnce(...). (IOU more details or an example.)

[dckc]

I think we'll want to use t.snapshot() for documentation.

By way of precendent, see
https://github.com/Agoric/agoric-sdk/tree/master/packages/inter-protocol#reading-data-off-chain

I think I used snapshots to make vstorage reference docs too, though I'm not sure I landed them on master.

[samsiegart]

Done

[dckc]

The simplest thing I can think of is hard-coding the policy in the transaction-feed.js exo sources.
But I don't suppose we know nobleAgoricChannelId at design-time; it varies between test nets.

The policy could be added to FastUSDCConfig.

init-fast-usdc.js is the UI for that config. It has a number of named configs for sets of oracle operator addresses. Or the addresses can be specified individually on the command-line.

A similar approach would work for chain policies, I suppose. For the ad-hoc case, great big blobs of JSON as command-line args are not out of the question. I don't think it can read files; it's not always run using agoric run. It's also invoked from cosmic-swingset somewhere with different endowments and no access to the filesystem.

[samsiegart]

Done. Just made the CLI accept stringified JSON for the policy.

[dckc]

It's a little strange that the caller provides the policy here... that is: that the transaction feed doesn't need the policy at start-up. This code could just go right in the creatorFacet. Or heck... it could go in fast-usdc.start.js.

[samsiegart]

Changed so that the start script writes the policy directly

[dckc]

Suggested change

	export const ChainPoliciesShape = {
	nobleContractAddress: EvmHashShape,
	cctpTokenMessengerAddress: EvmHashShape,
	confirmations: M.number(),
	chainName: M.string(),
	bridgeContractAddress: M.opt(EvmHashShape),
	eventFilter: M.opt(M.string()),
	chainType: M.opt(M.number()),
	isL2: M.opt(M.boolean()),
	};
	export const ChainPoliciesShape = M.splitRecord({
	nobleContractAddress: EvmHashShape,
	cctpTokenMessengerAddress: EvmHashShape,
	confirmations: M.number(),
	chainName: M.string(),
	},{
	bridgeContractAddress: EvmHashShape,
	eventFilter: M.string(),
	chainType: M.number(),
	isL2: M.boolean(),
	});

That way you don't need eventFilter: undefined. You can just leave the property out.

[samsiegart]

Done thanks

[dckc]

Is there any possibility that this is ever anything besides 4? Should we just constrain it?

Suggested change

	nobleDomainId: M.number(),
	nobleDomainId: 4,

[dckc]

See splitRecord() suggestion for the pattern; then we can skip these.

Suggested change

	bridgeContractAddress: undefined,
	eventFilter: undefined,
	chainType: undefined,
	isL2: undefined,

[0xpatrickdev]

Suggested change

	bridgeContractAddress?: EvmHash;
	eventFilter?: string;
	chainType?: number;
	isL2?: boolean;
	eventFilter?: string;
	chainType?: number;

I think we can exclude bridgeContractAddress and isL2 since there are no longer reqs to track L2 -> L1 confirmation.

[samsiegart]

Done

[0xpatrickdev]

Should we also include chainId: int;? This seems like a more authoritative identifier than chainName, albeit less human-readable.

Edit: I see chainId is used as a key. Maybe it's better to make chainName they key?

[samsiegart]

I just made chainName the key instead, since it's more self-describing. Having the chainId as the key might confuse people not knowing what the number means.

[0xpatrickdev]

I think this is the same for all chains, so we can maybe move this up to FeedPolicy

[samsiegart]

Done

[cloudflare-workers-and-pages]

Deploying agoric-sdk with    Cloudflare Pages

Latest commit:	aa40f20
Status:	✅  Deploy successful!
Preview URL:	https://b7cc4d8d.agoric-sdk.pages.dev
Branch Preview URL:	https://srs-chain-policy.agoric-sdk.pages.dev

View logs

[samsiegart]

Just changed it so the start script config accepts a FeedPolicy, and the script writes directly to vstorage rather than having a public facet method on the contract. PTAL.

[turadg]

Assuming the policy shape is correct, the code LGTM.

[turadg]

NTS: I've been assuming that published nodes have CapData but this will be raw JSON. Our client-utils should handle both

[dckc]

I have a few suggestions.

I think there's a conflict with #10532 as is. We'll see which one lands first :)

If #10532 doesn't land first, I can deal with stuff there.

[dckc]

This could be prettied up a bit using...

import { documentStorageSchema } from '@agoric/internal/src/storage-test-utils.js';

[samsiegart]

Done

[dckc]

It doesn't have a default, so it won't always be there.

Suggested change

	* feedPolicy: string;
	* feedPolicy?: string;

[samsiegart]

Done

[dckc]

This will crash if no --feedPolicy arg is given.

Not the end of the world, but "--feedPolicy is required" would be a nicer diagnostic than "cannot parse JSON"

[dckc]

Doesn't this make the a3p-integration test fail?

Oh... #10517 hasn't landed yet. sigh.

[samsiegart]

Happy to wait for that to go in, your call though.

[samsiegart]

Added a check for feedPolicy also

[dckc]

#10517 landed.

It has an --oracleSets flag. I suggest renaming that to --net. Then add a feedPolicy to each of the configurations.

[dckc]

I suggest a comment that this shape has to be JSON-happy: no bigint, undefined, remotable, etc.

[samsiegart]

Done.

[dckc]

This might read nicer using...

Suggested change

	'{"nobleAgoricChannelId":"channel-21","nobleDomainId":4,"chainPolicies":{"Arbitrum":{"cctpTokenMessengerAddress":"0x19330d10D9Cc8751218eaf51E8885D058642E08A","chainId":42161,"confirmations":1,"nobleContractAddress":"0x19330d10D9Cc8751218eaf51E8885D058642E08A"}}}',
	JSON.stringify({"nobleAgoricChannelId":"channel-21","nobleDomainId":4,"chainPolicies":{"Arbitrum":{"cctpTokenMessengerAddress":"0x19330d10D9Cc8751218eaf51E8885D058642E08A","chainId":42161,"confirmations":1,"nobleContractAddress":"0x19330d10D9Cc8751218eaf51E8885D058642E08A"}}}),

and then let the linter pretty-print it.

[samsiegart]

Done

[samsiegart]

PTAL

[samsiegart]

Done.

[samsiegart]

Happy to wait for that to go in, your call though.

[samsiegart]

Done

[samsiegart]

Done

[samsiegart]

Done

[samsiegart]

Added a check for feedPolicy also

[dckc]

looks fine.

[dckc]

oops. documentStorageSchema doesn't seem to grok the plain JSON style.

Oh well. Here's hoping it will in due course.

[dckc]

Assuming you follow the --net suggestion for init-fast-usdc.js:

Suggested change

	[
	'--oracle',
	'a:agoric1watcher1',
	'--feedPolicy',
	JSON.stringify({
	nobleAgoricChannelId: 'channel-21',
	nobleDomainId: 4,
	chainPolicies: {
	Arbitrum: {
	cctpTokenMessengerAddress:
	'0x19330d10D9Cc8751218eaf51E8885D058642E08A',
	chainId: 42161,
	confirmations: 1,
	nobleContractAddress:
	'0x19330d10D9Cc8751218eaf51E8885D058642E08A',
	},
	},
	}),
	],
	['--net', 'MAINNET']

like

agoric-sdk/packages/boot/test/bootstrapTests/price-feed-replace.test.ts

Line 104 in 6112a8b

buildProposal(priceFeedBuilder, ['MAINNET']),

[samsiegart]

Done. PTAL

[dckc]

looks right.

[dckc]

:)

[dckc]

--net and such looks fine