SPIKE: use offer result without zoe offer by dckc · Pull Request #11256 · Agoric/agoric-sdk

dckc · 2025-04-11T23:40:22Z

closes: #8733, #10926, #8550
refs:

governed contracts upgradable by electorate #7420

Description

This is an exploration of a design space...

delegate upgrade of a contract to an executive (toward governed contracts upgradable by electorate #7420)
- use contract facet only after upgrade
  - after endowment as in E.after()
- make nameHub available as my.XYZ
test(boot): set price without zoe offer using evalExpr
- feat(smart-wallet): save offer result under name given in offerSpec
- test(boot): save smartWallet offerResult under given name
- feat(smart-wallet): evalExpr (WIP)
  - justin syntax check is WIP: feat(parse): spruce up this package to make it useful endojs/Jessie#121 (comment)

test(boot): atomicSwap A<->B using smartWallet
- feat: deposit offer result (WIP) handle invitation offer results in smart wallet #8550
- WIP: current code has void E.when(walletHelper.receive(result), ???, ???) in a sync context. This looks like another state in offer handling.
- fix(vats): work around NameAdmin.readonly() return guard (WIP NEEDSTEST)
- feat(smart-wallet): endow evalExpr with namesByAddress so evalExpr can send to others
- refactor(boot): use actor style in atomicSwap test
- explore use of coveredCall in orchestration

feat: write access to the nameHub (NEEDSTEST)
endow evalExpr to make offers (subsume callPipe)

Security Considerations

promises as args can add a burden on the receiver

options for cost recovery / spam mitigation / credible governance:

pay a price to enable evalExpr
require staked BLD to use evalExpr
- go implementation of StakingView
  - cf. old x/lien, feat(run-protocol): RUNstake contract only, without payoff from rewards #4741
- require larger bond for some endowments
- does this suffice as governance for governed contracts upgradable by electorate #7420 ? or do we need a K-of-N electorate?
  - idea: contract for "if K of N send me the same Justin expr, I'll execute it"

Scaling Considerations

Justin expressions are designed to execute in O(N) in the size of the expressions, though using E can result in large constant factors -- cross-vat calls.

builtins such as [...].sort() use O(n log(N)) time

TODO:

measure cost savings for something like Fast USDC OCW submissions

Documentation Considerations

docs such as Submitting the Offer in the UI tutorial should get updated.

Testing Considerations

only basic happy-path testing is in place

Upgrade Considerations

does our keplr integration "just work" with evalExpr? If not, do we want/need to support it? Or do CLI tools suffice?

TODO:

proper smart wallet state migration

cloudflare-workers-and-pages · 2025-04-11T23:42:06Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`e3f38fc`
Status:	🚫 Build failed.

View logs

socket-security · 2025-04-11T23:42:10Z

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

michaelfig · 2025-04-12T23:37:00Z

packages/smart-wallet/src/smartWallet.js

+          // XXX worth using a nameHub vs. a mapStore?
+          const { nameHub, nameAdmin } = this.state.my;
+          const endowments = { E, harden, assert, nameHub, nameAdmin };
+          const c = new Compartment(endowments);


endowments are additions to the HardenedJS default allowed properties of c.globalThis. To restrict the Compartment's globalThis so that only endowments can be reached, I think you'd need something more like (untested):

Suggested change

const c = new Compartment(endowments);

const c = new Compartment();

for (const key of Reflect.ownKeys(c.globalThis)) {

assert(Reflect.deleteProperty(c.globalThis, key));

}

for (const [key, value] of Object.entries(endowments)) {

assert(Reflect.defineProperty(c.globalThis, key, { value }));

}

harden(c.globalThis);

I'm not sure about the scope of your suggestion... could you give an example of an expression that shows how things work differently with the suggested approach?

Given evaluation of a Justin expression:

try { const ret = c.evaluate(`Number.isSafeInteger(123)`); console.log('@@@ returned', ret); } catch (e) { console.log('@@@ caught', e); }

With the existing code, all of HardenedJS's tamed globals, in addition to the explicit endowments, are reachable by c.evaluate:

@@@ returned true

With (something like) my proposed code, only the endowments are reachable, not any other globals, so the above c.evaluate(...) will instead do:

@@@ caught ReferenceError: Number is not defined

I suggest you indicate in a comment which of these behaviours you desire for the c.evaluate.

It seems like Number.isSafeInteger should be available. Is there anything in the default environment of Compartment that carries authority?

I suppose compute time is relevant too... [...].sort() will be O(n log(n)) rather than the usual case of O(n) for Justin exprs. I'm not sure denying that is practical, though. hm.

anything in... Compartment that carries authority?

Not in the sense of the ability to break integrity, short of what is added to c.globalThis (such as the properties of additionalGlobals in new Compartment(additionalGlobals)).

I'm not sure denying that is practical

Just spitballing some ideas, neither of which is necessarily practical or desirable:

Further restrict an individual Compartment's capabilities with an allowlist or membrane, but both of those require HardenedJS features that we don't have yet.

Have the host vat spawn separate child vats to perform evaluations, but that doesn't scale well in the current SwingSet.

Talking with @erights, it seems there's a sweet spot I hadn't considered:

Have a multitenant vat provide a hand-written Justin interpreter:

The interpreter can take a method allowlist as a parameter, and also have restricted implementations of JS builtins

Interpreter-level metering can abort a particular evaluation long before an XS-level meter overflow would kill the vat

@michaelfig , I agree with the first bullet.

On the second bullet, yes, with care.

Normally we allow the Justin program to cause synchronous side effects. But if you terminate it with any meter-like mechanism and you don't kill the vat it is affecting, then you leave behind inconsistent corrupt state. Unless you

limit the Justin code to only performing synchronous invocations that it could have performed remotely and asynchronously.

start the Justin execution in its own turn.

Then, any of the methods it could invoke are designed to transition the vat from a consistent state to a consistent state by itself, without making assumptions about future incoming messages causing other effects.

IOW, sudden termination of such a local Justin computation is equivalent to a remote invoker who stops.

dckc · 2025-04-18T18:19:06Z

packages/smart-wallet/src/smartWallet.js

+            assert,
+            nameHub,
+            nameAdmin,
+            namesByAddress,


watch out for promises as arguments.

WIP: state migration (upgrade)

WIP: validate Justin syntax

Logging sent error stack Error: In "readonly" method of (NamesByAddressAdmin): result: promise "[Promise]" - Must be a remotable at makeError (file:///home/connolly/projects/agoric-sdk/node_modules/ses/src/error/assert.js:350:61) at throwLabeled (.../common/throw-labeled.js:23:20) at applyLabelingError (.../common/apply-labeling-error.js:43:5) at checkMatches (.../patterns/src/patterns/patternMatchers.js:420:5) at mustMatch (.../patterns/src/patterns/patternMatchers.js:596:5) at In "readonly" method of (NamesByAddressAdmin) [as readonly] (.../exo/src/exo-tools.js:173:11)

returns obj after rx[method](...args) resolves

- price contract tracks incarnation - carrier contract delivers prize and shuts down - get boardId for carrier instance without putting it in agoricNames - startContract() handles privateArgs - move name, issuers to options bag - different friction around type of startSwap - use my.XYZ instead of nameHub.lookup(...); e.g. my.priceSetter

dckc · 2025-04-22T00:17:30Z

packages/boot/test/smartWallet/wallet-fun.test.ts

+          E(
+            after(my.kit.publicFacet, {
+              rx: my.kit.adminFacet as any, // XXX after() type
+              method: 'restartContract',
+              args: [my.kit.privateArgs],
+            }),
+          ).getIncarnation(); // use the public (TODO: creator) facet


I learned in discussion with @kriskowal that this is needlessly contorted; it could/should be:

after(my.kit.publicFacet, E(my.kit.adminFacet).restartContract(my.kit.privateArgs)).getIncarnation()

with...

const after = (obj, prereq) = E.when(prereq, () => obj)

dckc · 2025-04-22T00:19:53Z

packages/smart-wallet/src/smartWallet.js

+        async evalExpr(expr) {
+          const { fromEntries } = Object;
+          trace(
+            'TODO: validate Justin syntax; see https://github.com/endojs/Jessie/pull/121#discussion_r1988126110',


@michaelfig while Justin is designed to execute in O(N), here we'd be parsing it. The PEG parser... does it backtrack? That wouldn't be O(N), would it?

discovered in discussion with @kriskowal

PEG is the bnf-like grammar definition language. Packrat is the generated parsing algorithm. Due to its heavy memoization, for most purposes, packrat is linear in time and space. See the original paper at Packrat Parsing:
Simple, Powerful, Lazy, Linear Time, or the wikipedia page.

From Bryan Ford's paper:

The basic reason the backtracking parser can take super-linear
time is because of redundant calls to the same parse function on the
same input substring, and these redundant calls can be eliminated
through memoization.

refs: - #8733 - #11256 ## Description - add a `nameHub` and `nameAdmin` to state of each offer result - add `spec.after.saveAs` option in `OfferSpec` to save an offer result - add `invokeItem` method in `BridgeAction` that uses a format somewhat like `callPipe` DRAFT until the TODOs below are addressed. ### Security / Scaling Considerations This is intended to serve as an efficient, straightforward alternative to continuing offers for the (common) case when no asset exchange is happening. The motivating usage is ymax oracles (it facitiliates optimizing Fast USDC oracles too). TODO: - [x] think thru allowing everyone, not just oracles, to use `invokeItem`. Without a suitable invitation, it's useless; but what about griefing? - [x] `callPipe` was allowed only on `agoricContract`; are we ready to drop that limitation? - [x] length limit was 3; this is currently unlimited; we haven't found motivation to extend past 3, have we? ### Documentation / Testing Considerations One happy path test shows that the feature works and how to use it. TODO - [x] test failure modes - [x] the test uses bundled code, which doesn't mix well with coverage tools. Invest in testBundle tests for smartWallet? ### Upgrade Considerations TODO: - [ ] figure out upgrade implications of adding to smartWallet exo state

dckc · 2025-09-29T15:45:16Z

OBE

dckc force-pushed the dc-wallet-justin branch 2 times, most recently from 2127774 to 315c79e Compare April 12, 2025 03:07

michaelfig reviewed Apr 12, 2025

View reviewed changes

dckc force-pushed the dc-wallet-justin branch from 315c79e to b8cf5b4 Compare April 15, 2025 17:33

dckc commented Apr 18, 2025

View reviewed changes

packages/smart-wallet/src/smartWallet.js

assert,

nameHub,

nameAdmin,

namesByAddress,

Copy link

Member Author

dckc Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

watch out for promises as arguments.

dckc changed the title ~~use offer result without zoe offer~~ SPIKE: use offer result without zoe offer Apr 19, 2025

dckc added 10 commits April 18, 2025 21:31

feat(smart-wallet): save offer result under given name (WIP)

a61dddc

WIP: state migration (upgrade)

test(boot): save smartWallet offerResult under given name

df10699

feat(smart-wallet): evalExpr (WIP)

e7cf6f1

WIP: validate Justin syntax

test(boot): set price without zoe offer using evalExpr

f62be3d

feat: deposit offer result (WIP. NEEDSTEST)

89877df

test(boot): deposit Invitation offer result

4be743b

feat(smart-wallet): endow evalExpr with namesByAddress

70bc8e7

test(boot): atomicSwap A<->B using smartWallet

e29aa8f

refactor(boot): use actor style in atomicSwap test

3dc72bf

dckc force-pushed the dc-wallet-justin branch from b8cf5b4 to fb1a040 Compare April 19, 2025 02:32

dckc added 3 commits April 19, 2025 00:02

feat(smartWallet): make nameHub available as my.XYZ

f2bb402

feat(smartWallet): after(obj, { rx, method, args })

38bf4cd

returns obj after rx[method](...args) resolves

dckc force-pushed the dc-wallet-justin branch from fb1a040 to ab873a7 Compare April 19, 2025 05:02

dckc added 7 commits April 19, 2025 13:17

refactor: move wallet-fun test to new dir

f77d73c

WIP: carrier contract

26ed395

chore: provide stakingView in walletFactoryContext

9739fb1

refactor(smartWallet): use E.when for after()

ce2cf3b

feat(smartWallet): publish passStyle of evalExpr result

d03544b

feat(smart-wallet): require 10K staked BLD to evalExpr

5130306

chore(boot): support stakingView in walletDriver

597ce1f

test(boot): require staked BLD for evalExpr

e3f38fc

dckc force-pushed the dc-wallet-justin branch from c4647af to e3f38fc Compare April 19, 2025 20:15

dckc added the wallet label Apr 21, 2025

dckc commented Apr 22, 2025

View reviewed changes

dckc mentioned this pull request Jul 28, 2025

feat(smartWallet): invoke offerResult without involving Zoe #11672

Merged

6 tasks

dckc closed this Sep 29, 2025

-          const c = new Compartment(endowments);
+          const c = new Compartment();
+          for (const key of Reflect.ownKeys(c.globalThis)) {
+            assert(Reflect.deleteProperty(c.globalThis, key));
+          }
+          for (const [key, value] of Object.entries(endowments)) {
+            assert(Reflect.defineProperty(c.globalThis, key, { value }));
+          }
+          harden(c.globalThis);

Comments

Conversation

dckc commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

Uh oh!

cloudflare-workers-and-pages bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying agoric-sdk with Cloudflare Pages

Uh oh!

socket-security bot commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erights Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dckc commented Sep 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dckc commented Apr 11, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Apr 11, 2025 •

edited

Loading

socket-security bot commented Apr 11, 2025 •

edited

Loading

erights Apr 22, 2025 •

edited

Loading