Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

chore: upgrade capi to gamma.1 #217

Closed
wants to merge 16 commits into from
Closed

chore: upgrade capi to gamma.1 #217

wants to merge 16 commits into from

Conversation

peetzweg
Copy link
Member

capi sync does not work as expected anymore. Had this issue already with beta43, any ideas how to fix this @harrysolovay ?

Screenshot 2023-06-16 at 11 59 36

Without --runtime-config package.json:

Screenshot 2023-06-16 at 12 00 16

@netlify
Copy link

netlify bot commented Jun 16, 2023

Deploy Preview for capi-multisig ready!

Name Link
🔨 Latest commit 30976d5
🔍 Latest deploy log https://app.netlify.com/sites/capi-multisig/deploys/64a6757fd4a84a0008e51c3a
😎 Deploy Preview https://deploy-preview-217--capi-multisig.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@peetzweg peetzweg changed the title core: upgrade capi to beta47 chore: upgrade capi to beta47 Jun 16, 2023
@peetzweg
Copy link
Member Author

Also Rune.run() type might be broken? It expects a required argument scope now?

Screenshot 2023-06-16 at 12 10 23

@kratico
Copy link
Contributor

kratico commented Jun 16, 2023

@peetzweg for the capi sync issue, the nets.ts file needs to be updated with

diff --git a/nets.ts b/nets.ts
index 05a9c15..e81c2e4 100644
--- a/nets.ts
+++ b/nets.ts
@@ -1,4 +1,4 @@
-import { net } from "capi"
+import { net } from "capi/nets"
 
 export const westend = net.ws({
   url: "wss://westend-rpc.polkadot.io/",

@peetzweg
Copy link
Member Author

import { net } from "capi/nets"

Ah alright, wasn't aware of that change. Does not seem to be resolved correctly though.

Screenshot 2023-06-16 at 13 21 38

I've tried as well:
capi/nets
capi/nets/mod.js
capi/nets/mod

@kratico
Copy link
Contributor

kratico commented Jun 16, 2023

Yeap, this is weird and it's related to the type declaration.
Despite the type issue, I tried capi sync and it worked.

Did capi sync work for you?

I tweaked this manually in the package.json by rewriting the subpath export and it seems to work
image

Created the issue paritytech/capi#1088

@peetzweg
Copy link
Member Author

Ignoring this type issue here. In development the app does not load anymore and this error Rune related error shows up in the console.

Screenshot 2023-06-16 at 14 01 51

@tjjfvi
Copy link

tjjfvi commented Jun 16, 2023

Also Rune.run() type might be broken? It expects a required argument scope now?

This is to be expected (only by us, that is, seeing as we failed to let you know); as of paritytech/capi#1084 this argument is required.

You can declare

const scope = new Scope()

at the top-level of your app, and pass that in to every run. This will ensure that all execution happen in the same scope, thus sharing cache etc.

(Eventually, we'll likely make a useRune hook, which – among other things – will handle passing in the scope via context)

Ignoring this type issue here. In development the app does not load anymore and this error Rune related error shows up in the console.

Once you pass in a scope this should be resolved.

@peetzweg peetzweg requested a review from tjjfvi June 16, 2023 13:26
@peetzweg
Copy link
Member Author

peetzweg commented Jun 16, 2023

I think solving it in a "react" library down the line is quite far away, making this use very cumbersome.

How about setting a "defaultScope" for all Runes in a static way and just pass an override scope on a per rune basis?

Haven't looked into the details of the scope but feels like something which is not changing too often in the lifecylce of an app, or is it?

@tjjfvi
Copy link

tjjfvi commented Jun 16, 2023

I'm not sure how you define quite far away, but I could see working on such a library before v0.2.0.

We discussed having a default, global scope, but decided that this would be too much of a footgun, as it would be very easy to forget to pass in an override scope.

@peetzweg peetzweg requested review from statictype and removed request for kratico, harrysolovay, ryanleecode and tjjfvi June 19, 2023 10:40
@peetzweg
Copy link
Member Author

Ready for review.

server/scripts/send-transfer.ts Outdated Show resolved Hide resolved
www/src/signals/scope.ts Outdated Show resolved Hide resolved
@harrysolovay
Copy link
Contributor

harrysolovay commented Jun 19, 2023

For the time being, I would just pass an anonymous scope into every run. No need to persist in a signal. We'll revisit this after addressing paritytech/capi#1091

@peetzweg peetzweg requested a review from statictype June 19, 2023 15:05
@peetzweg peetzweg changed the title chore: upgrade capi to beta47 chore: upgrade capi to gamma.1 Jul 5, 2023
@peetzweg peetzweg mentioned this pull request Jul 5, 2023
@peetzweg
Copy link
Member Author

peetzweg commented Jul 5, 2023

Just updated to gamma.1, works fine so far. Like creating a proxy, replacing delegates and funding. But in ratify / cancel multisig extrinsics I get the following errors on the command line cc @tjjfvi @harrysolovay .

I have not yet isolated the issue so not created a capi issue for it. Maybe it's obvious how to quickfix it?

The actual extrinsic seems to have made it through though.

Screenshot 2023-07-05 at 09 48 47

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@statictype
Copy link
Contributor

i'm handling the update in a new PR. this branch is too old, don't want to fix all these conflicts

@statictype statictype closed this Jul 6, 2023
@peetzweg
Copy link
Member Author

peetzweg commented Jul 6, 2023

Just resolved all conflicts minutes before. 🤷 Most of the changes are just formatting stuff anyways. Btw we have dprint and prettier in this repo. I think we should drop one of them to prevent this formatting ping pong.

"prettier": "^2.8.8",

"plugins": [
"https://plugins.dprint.dev/json-0.17.0.wasm",
"https://plugins.dprint.dev/markdown-0.15.2.wasm",
"https://plugins.dprint.dev/typescript-0.83.0.wasm"
]

@statictype
Copy link
Contributor

wouldn't merge this version now because it produces an error with approvals and cancellations. even if the tx goes through, we can't capture the events anymore.

@harrysolovay harrysolovay deleted the chore-capi-beta47 branch July 6, 2023 13:12
@harrysolovay harrysolovay restored the chore-capi-beta47 branch July 6, 2023 13:12
@harrysolovay
Copy link
Contributor

paritytech/capi#1139

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants