Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

empty diagnostic method list from amm public facet etc. #6570

Closed
dckc opened this issue Nov 15, 2022 · 15 comments · Fixed by endojs/endo#1382
Closed

empty diagnostic method list from amm public facet etc. #6570

dckc opened this issue Nov 15, 2022 · 15 comments · Fixed by endojs/endo#1382
Assignees
Labels
bug Something isn't working contract-upgrade devex developer experience Inter-protocol Overarching Inter Protocol vaults_triage DO NOT USE
Milestone

Comments

@dckc
Copy link
Member

dckc commented Nov 15, 2022

Describe the bug

The "no method" diagnostic from the AMM public facet used to include the list of names of methods that it does have. Now that list is empty.

To Reproduce

In a REPL:

command[10] E(home.agoricNames).lookup('instance', 'amm')
history[10] [Object Alleged: InstanceHandle]{}
command[11] E(home.zoe).getPublicFacet(h[10])
history[11] [Object Alleged: AMM publicFacet]{}
command[12] E(h[11]).help()
history[12] Promise.reject("TypeError: target has no method \"help\", has []")

Expected behavior

The has [] list should include the available methods; for example:

command[13] E(home.zoe).getPublicFacet(E(home.agoricNames).lookup('instance', 'Pegasus'))
history[13] [Object Alleged: pegasus]{}
command[14] E(h[13]).help()
history[14] Promise.reject("TypeError: target has no method \"help\", has [\"getLocalIssuer\",\"makeInvitationToTransfer\",\"makePegasusConnectionKit\"]")

Platform Environment

  • what OS are you using? what version of Node.js? v16.17.0
  • is there anything special/unusual about your platform?
  • what version of the Agoric-SDK are you using? (run git describe --tags --always)

@agoric/[email protected]

Additional context

I suspect this is due to wrapping the AMM public facet in virtual / durable API stuff.

cc @michaelfig @Chris-Hibbert @turadg

@dckc dckc added bug Something isn't working devex developer experience Inter-protocol Overarching Inter Protocol contract-upgrade labels Nov 15, 2022
@Chris-Hibbert
Copy link
Contributor

The AMM's publicFacet is built with augmentVirtualPublicFacet, which is defined in contractHelper.js as

  const augmentVirtualPublicFacet = originalPublicFacet => {
    return Far('publicFacet', {
      ...originalPublicFacet,
      ...commonPublicMethods,
      ...objectMap(typedAccessors, ignoreContext),
    });
  };

This hasn't changed recently, and I thought Far classes were more self-revealing than that. Does that Alleged AMM publicFacet respond to the actual AMM public methods, like getLiquiditySupply, getQuoteIssuer, or getAllPoolBrands?

@dckc
Copy link
Member Author

dckc commented Nov 15, 2022

... Does that Alleged AMM publicFacet respond to the actual AMM public methods, like getLiquiditySupply, getQuoteIssuer, or getAllPoolBrands?

yes:

command[15] E(h[11]).getAllPoolBrands()
history[15] [[Object Alleged: ATOM brand]{},[Object Alleged: BLD brand]{},[Object Alleged: LINK brand]{},[Object Alleged: USDC brand]{},[Object Alleged: WETH brand]{}]

ignoreContext seems sort of magical... I wonder if it introduces a conflict somehow.

@michaelfig
Copy link
Member

The code that displays the diagnostic uses Reflect.ownKeys(obj) to fetch the properties. Very likely with the introduction of Far Classes, method properties are moved somewhere else in the prototype chain (not as "own" properties of the object identity).

@erights, do you care to weigh in on this?

@Chris-Hibbert
Copy link
Contributor

ignoreContext seems sort of magical... I wonder if it introduces a conflict somehow.

ignoreContext is just taking functions of N arguments and dropping the first arg before calling them. It shouldn't have an impact like this.

@dckc
Copy link
Member Author

dckc commented Nov 30, 2022

search keyword: help

@dckc
Copy link
Member Author

dckc commented Dec 5, 2022

Good to see that land, but I think this needs an endo update before it's fixed.

@dckc dckc reopened this Dec 5, 2022
@erights
Copy link
Member

erights commented Jan 30, 2023

@dckc do you agree that this is fixed by endojs/endo#1382 and can be closed?

I think it wasn't closed at the time only because it agoric-sdk until upgraded to depend on an endo post that PR, which happened long ago.

@dckc
Copy link
Member Author

dckc commented Jan 30, 2023

No; I just tried the REPL again, and the symptoms persist:

command[0] E(agoric.agoricNames).lookup('instance', 'amm')
history[0] [Object Alleged: InstanceHandle]{}
command[1] h=history,0
history[1] 0
command[2] ...
command[3] amm = {instance: h[0]}
history[3] {"instance":[Object Alleged: InstanceHandle]{}}
command[4] E(home.zoe).getPublicFacet(amm.instance)
history[4] [Object Alleged: AMM publicFacet]{}
command[5] amm.pub = h[4]
history[5] [Object Alleged: AMM publicFacet]{}
command[6] E(amm.pub).help()
history[6] Promise.reject("TypeError: target has no method \"help\", has []")

I don't see anything useful in the chain-side stack trace:

2023-01-30T17:07:43.223Z SwingSet: ls: v37: Logging sent error stack (TypeError#1)
2023-01-30T17:07:43.223Z SwingSet: ls: v37: TypeError#1: target has no method help , has []
2023-01-30T17:07:43.223Z SwingSet: ls: v37: TypeError: target has no method "help", has []
 at makeError (/bundled-source/.../node_modules/ses/src/error/assert.js:242)
 at fail (/bundled-source/.../node_modules/ses/src/error/assert.js:370)
 at localApplyMethod (/bundled-source/.../node_modules/@endo/eventual-send/src/local.js:116)
 at apply ()
 at dispatchToHandler (/bundled-source/.../node_modules/@endo/eventual-send/src/handled-promise.js:163)
 at win (/bundled-source/.../node_modules/@endo/eventual-send/src/handled-promise.js:488)
 at ()

2023-01-30T17:07:43.223Z SwingSet: ls: v37: TypeError#1 ERROR_NOTE: Sent as error:liveSlots:v37#70001

@dckc
Copy link
Member Author

dckc commented Jan 30, 2023

Odd... it works for some (home.agoricNames, ...) but not for others (home.zoe, ...):

command[7] E(home.zoe).help()
history[7] Promise.reject("TypeError: target has no method \"help\", has []")

command[8] E(home.agoricNames).help()
history[8] Promise.reject("TypeError: target has no method \"help\", has [\"entries\",\"keys\",\"lookup\",\"values\"]")
command[9] Object.keys(agoric)
history[9] ["agoricNames","attMaker","bank","board","chainTimerService","faucet","ibcport","myAddressNameAdmin","namesByAddress","priceAuthority","priceAuthorityAdminFacet","vaultFactoryCreatorFacet","zoe"]
command[10] E(home.priceAuthority).help()
history[10] Promise.reject("TypeError: target has no method \"help\", has [\"getQuoteIssuer\",\"getTimerService\",\"makeQuoteNotifier\",\"mutableQuoteWhenGT\",\"mutableQuoteWhenGTE\",\"mutableQuoteWhenLT\",\"mutableQuoteWhenLTE\",\"quoteAtTime\",\"quoteGiven\",\"quoteWanted\",\"quoteWhenGT\",\"quoteWhenGTE\",\"quoteWhenLT\",\"quoteWhenLTE\"]")
...
command[14] E(agoric.attMaker).help()
history[14] Promise.reject("TypeError: target has no method \"help\", has []")
command[15] E(agoric.board).help()
history[15] Promise.reject("TypeError: target has no method \"help\", has [\"getId\",\"getPublishingMarshaller\",\"getReadonlyMarshaller\",\"getValue\",\"has\",\"ids\",\"lookup\"]")
command[16] E(agoric.bank).help()
history[16] Promise.reject("TypeError: target has no method \"help\", has [\"getAssetSubscription\",\"getPurse\"]")
command[17] E(agoric.chainTimerService).help()
history[17] Promise.reject("TypeError: target has no method \"help\", has []")
command[18] E(agoric.faucet).help()
history[18] Promise.reject("TypeError: target has no method \"help\", has [\"tapFaucet\"]")

command[19] E(agoric.agoricNames).lookup('instance', 'VaultFactory').then(i => E(home.zoe).getPublicFacet(i).then(vfp => E(vfp).help()))
history[19] Promise.reject("TypeError: target has no method \"help\", has []")

@erights
Copy link
Member

erights commented Jan 30, 2023

command[7] E(home.zoe).help()
history[7] Promise.reject("TypeError: target has no method \"help\", has []")

That is extremely odd. I don't understand how that could happen. Definitely need to investigate. I will leave this bug open. Thanks!

@michaelfig
Copy link
Member

That is extremely odd.

Did you consider my reasoning in #6570 (comment) ?

@erights
Copy link
Member

erights commented Jan 30, 2023

That is extremely odd.

Did you consider my reasoning in #6570 (comment) ?

I did. That is what endojs/endo#1382 was supposed to fix. https://github.com/endojs/endo/blob/1d142e58bc3a7388cf851fc74892707f73e4cfd1/packages/eventual-send/src/local.js#L59 is where it ascends the prototype chain. Is the algorithm containing this line wrong, or is the problem elsewhere?

@michaelfig
Copy link
Member

or is the problem elsewhere?

We're using different code! That version of @endo/eventual-send has not yet been adopted by agoric-sdk.

@erights
Copy link
Member

erights commented Mar 2, 2023

Would be fixed by fixing #7090

@ivanlei
Copy link
Contributor

ivanlei commented Apr 17, 2023

This was fixed after #7090 landed

command[3]
E(home.zoe).help()
history[3]
Promise.reject("TypeError: target has no method \"help\", has [\"getBrands\",\"getBundleIDFromInstallation\",\"getConfiguration\",\"getFeeIssuer\",\"getInstallation\",\"getInstallationForInstance\",\"getInstance\",\"getInvitationDetails\",\"getInvitationIssuer\",\"getIssuers\",\"getOfferFilter\",\"getProposalShapeForInvitation\",\"getPublicFacet\",\"getTerms\",\"install\",\"installBundleID\",\"offer\",\"startInstance\"]")

@ivanlei ivanlei closed this as completed Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working contract-upgrade devex developer experience Inter-protocol Overarching Inter Protocol vaults_triage DO NOT USE
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants