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

E(namesByAddressAdmin).readonly() throws pattern error: promise vs. remotable #8408

Open
sufyaankhan opened this issue Sep 28, 2023 · 4 comments · Fixed by #8821
Open

E(namesByAddressAdmin).readonly() throws pattern error: promise vs. remotable #8408

sufyaankhan opened this issue Sep 28, 2023 · 4 comments · Fixed by #8821
Labels
bug Something isn't working namehub-petname issuer naming, petnames, namehubs, ...

Comments

@sufyaankhan
Copy link

Describe the bug

I was running the agoric-upgrade-11wf and faced the patternMatcher problem of the E(namesByAddressAdmin).readonly() again.

Here's the stack trace:

2023-09-27T19:15:37.565Z SwingSet: ls: v11: Logging sent error stack (Error#1)
2023-09-27T19:15:37.565Z SwingSet: ls: v11: Error#1: In "readonly" method of (NamesByAddressAdmin): result: promise (an object) - Must be a remotable
2023-09-27T19:15:37.565Z SwingSet: ls: v11: Error: In "readonly" method of (NamesByAddressAdmin): result: promise (an object) - Must be a remotable
 at construct ()
 at Error (/bundled-source/.../node_modules/ses/src/error/tame-error-constructor.js:56)
 at makeError (/bundled-source/.../node_modules/ses/src/error/assert.js:243)
 at throwLabeled (/bundled-source/.../node_modules/@endo/patterns/src/utils.js:132)
 at applyLabelingError (/bundled-source/.../node_modules/@endo/patterns/src/utils.js:154)
 at mustMatch (/bundled-source/.../node_modules/@endo/patterns/src/patterns/patternMatchers.js:528)
 at In "readonly" method of (NamesByAddressAdmin) (/bundled-source/.../node_modules/@endo/exo/src/exo-tools.js:43)
 at apply ()
 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:511)
 at ()

We need to send some invitations the core-eval ; And we can't get the depositFacets for those accounts

e.g.
const namesByAddress = E(namesByAddressAdmin).readonly()
patternMatcher throws error.
It says the method 'readonly' should return a remotable but it returns a promise

Developer used the following fix to get it working in their local environment but it did not work in DevNet

readonly: M.call().returns(M.remotable()),

(change it to readonly: M.call().returns(M.any())

To Reproduce

Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior

A clear and concise description of what you expected to happen.

Platform Environment

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

Additional context

Read the full thread discussion
https://discord.com/channels/585576150827532298/755164472397660313/1156672044608532590

Screenshots

If applicable, add screenshots to help explain your problem, especially for UI interactions.

@sufyaankhan sufyaankhan added the bug Something isn't working label Sep 28, 2023
@mhofman
Copy link
Member

mhofman commented Sep 28, 2023

@michaelfig could you clarify in which vat this code is actually loaded? You mentioned it was used in the bootstrap vat, but #4548 seem to indicate it was moved to the provisioning vat, which seem to match the v11 from the error stack.

Regardless, this is the kind of fix that requires a vat upgrade, and as such depends on #8219 to reach mainnet. Note that if instead of provisioning this lives in the bootstrap vat, then it would not be addressable on mainnet without some heroics since the bootstrap vat is not upgradable. It may be possible to migrate most powers from the bootstrap vat into a new vat however, but in some case that relies on informing upgraded vats they need to use a new object exported by another vat.

@michaelfig
Copy link
Member

@sufyaankhan, the workaround that doesn't need any code changes on devnet or mainnet is:

const addr = 'agoric1abcdeffoobarbaz...';
await E(namesByAddressAdmin).reserve(addr);
// don't trigger the namesByAddressAdmin.readonly() bug
const addressAdmin = E(namesByAddressAdmin).lookupAdmin(addr);
await E(addressAdmin).reserve('depositFacet');
const addressHub = E(addressAdmin).readonly();
const addressDepositFacet = E(addressHub).lookup('depositFacet');

Folks can trace through reserveThenDeposit in packages/inter-protocol/src/proposals/utils.js for the code that this snippet is based on. Alternatively, they can import that file and use reserveThenDeposit directly.

@michaelfig
Copy link
Member

@mhofman, yes the namesByAddressAdmin object is in the provisioning vat, v11.

@dckc dckc changed the title patternMatcher throws error E(namesByAddressAdmin).readonly() throws pattern error: promise vs. remotable Sep 30, 2023
@dckc dckc added the namehub-petname issuer naming, petnames, namehubs, ... label Jan 19, 2024
@mergify mergify bot closed this as completed in #8821 Jan 27, 2024
@dckc
Copy link
Member

dckc commented Mar 21, 2024

I think I reduced the scope of #8821 so it didn't close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working namehub-petname issuer naming, petnames, namehubs, ...
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants