Orchestration-basics v1 release #10
Conversation
…ment to starship environment
| > | ||
| {loadingCreateAccount ? ( | ||
| <svg aria-hidden="true" role="status" className="inline w-4 h-4 me-3 text-gray-200 animate-spin dark:text-gray-600" viewBox="0 0 100 101" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <path d="M100 50.5908C100 78.2051 77.6142 100.591 50 100.591C22.3858 100.591 0 78.2051 0 50.5908C0 22.9766 22.3858 0.59082 50 0.59082C77.6142 0.59082 100 22.9766 100 50.5908ZM9.08144 50.5908C9.08144 73.1895 27.4013 91.5094 50 91.5094C72.5987 91.5094 90.9186 73.1895 90.9186 50.5908C90.9186 27.9921 72.5987 9.67226 50 9.67226C27.4013 9.67226 9.08144 27.9921 9.08144 50.5908Z" fill="currentColor"/> |
There was a problem hiding this comment.
where do these path numbers come from? how do we maintain this?
There was a problem hiding this comment.
this is just a raw svg path for a common spinner. I was going to sub it out for a lib component, prob from daisy since its a dep
There was a problem hiding this comment.
It's often OK to copy from an open source package, but we need to acknowledge the source. And we need to check that the license isn't copyleft; it has to be compatible with ours (Apache 2).
| const RpcEndpoints = { | ||
| osmosis: 'http://127.0.0.1:26655', | ||
| agoric: 'http://127.0.0.1:26657', | ||
| }; |
There was a problem hiding this comment.
this doesn't look like it's indented in the conventional style. do we have yarn lint set up in this repo?
There was a problem hiding this comment.
did you say you got yarn lint working? I'm struggling to get it to work:
10:13 connolly@bldbox$ yarn lint
Unknown Syntax Error: Command not found; did you mean one of:
version I have checked out is:
- 2024-08-13 15:31 919cf28 create buttons are now a group for both handleCreateAccount and handleCreateAndFund
There was a problem hiding this comment.
hm, that commit should work for you. what directory are you running the yarn lint from? @dckc
You can run this from TLD:
yarn workspace dapp-agoric-orca-contract lint
or simply
make lint
or from /contract, you can just run yarn lint
result
ontract % yarn workspace dapp-agoric-orca-contract lint
/Users/jovonni/Documents/projects/devtes/dapp-orchestration-basics/contract/src/orca.contract.js
160:33 warning The first `await` appearing in an async function must not be nested @jessie.js/safe-await-separator
/Users/jovonni/Documents/projects/devtes/dapp-orchestration-basics/contract/src/orca.proposal.js
35:1 warning Missing JSDoc @param "config" type jsdoc/require-param-type
/Users/jovonni/Documents/projects/devtes/dapp-orchestration-basics/contract/src/platform-goals/start-contract.js
39:1 warning Missing JSDoc @param "privateArgs" type jsdoc/require-param-type
✖ 3 problems (0 errors, 3 warnings)
There was a problem hiding this comment.
yarn lint from tld throws:
dapp-orchestration-basics % yarn lint
Unknown Syntax Error: Command not found; did you mean one of:
0. yarn workspaces list [--since] [-R,--recursive] [--no-private] [-v,--verbose] [--json]
1. yarn workspaces focus [--json] [--production] [-A,--all] ...
2. yarn workspaces foreach [--from #0] [-A,--all] [-R,--recursive] [-W,--worktree] [-v,--verbose] [-p,--parallel] [-i,--interlaced] [-j,--jobs #0] [-t,--topological] [--topological-dev] [--include #0] [--exclude #0] [--no-private] [--since] [-n,--dry-run] <commandName> ...
While running workspaces run lint
jovonni@jovonnis-MBP dapp-orchestration-basics %
| return; | ||
| } | ||
|
|
||
| const { instances } = useContractStore.getState(); |
There was a problem hiding this comment.
the name instances is a little surprising... it's instance in agoricNames (that's orthogonal to this PR, so IOU an issue)
| } | ||
| if (update.status === 'accepted') { |
There was a problem hiding this comment.
no else? can update.status change in the if above?
does it make sense to use switch(update.status)?
| default: | ||
| return ''; |
There was a problem hiding this comment.
this looks like a silent failure. is that by design?
|
|
||
| const rpcEndpoint = rpcEndpoints[chain]; | ||
| try { | ||
| const balance = await fetchBalanceFromRpc(address, rpcEndpoint); |
There was a problem hiding this comment.
doesn't this trigger a "used before defined" eslint error?
| }; | ||
|
|
||
| const fetchBalanceFromRpc = async (address, rpcEndpoint) => { | ||
| const client = await StargateClient.connect(rpcEndpoint); |
There was a problem hiding this comment.
| @@ -0,0 +1,44 @@ | |||
| export const initializeKeplr = async () => { | |||
| await window.keplr.experimentalSuggestChain({ | |||
There was a problem hiding this comment.
how about passing in window.keplr explicitly rather than using ambient authority?
modules should not export powerful objects (for example, objects that close over
fsorprocess.env)
-- OCap Discipline
window and window.keplr are in the same class as fs and process and process.env.
There was a problem hiding this comment.
another possibility is to export the data and call window.keplr.experimentalSuggestChain elsewhere.
| agoric: 'http://127.0.0.1:26657', | ||
| }; | ||
|
|
||
| export default RpcEndpoints; |
There was a problem hiding this comment.
missing newline at end of file... again, do we have yarn lint set up here?
| @@ -1,9 +1,8 @@ | |||
| import { create } from 'zustand'; | |||
|
|
|||
| interface ContractState { | |||
| instance?: unknown; | |||
| instances?: Record<string, unknown>; | |||
There was a problem hiding this comment.
oh... so it's not orthogonal to this PR. elsewhere this is called instance; i.e. agoricNames.instance.orca. (likewise brand, issuer, ...)
OTOH, in the Zoe API, getTerms() returns something with issuers and brands... so... go figure.
| "bn.js@npm:^5.2.0": "patch:bn.js@npm%3A4.12.0#~/.yarn/patches/bn.js-npm-4.12.0-3ec6c884f6.patch", | ||
| "bn.js@npm:^4.11.9": "patch:bn.js@npm%3A4.12.0#~/.yarn/patches/bn.js-npm-4.12.0-3ec6c884f6.patch", | ||
| "bn.js@npm:^5.2.1": "patch:bn.js@npm%3A4.12.0#~/.yarn/patches/bn.js-npm-4.12.0-3ec6c884f6.patch", | ||
| "bn.js@npm:^4.11.8": "patch:bn.js@npm%3A4.12.0#~/.yarn/patches/bn.js-npm-4.12.0-3ec6c884f6.patch", | ||
| "bn.js@npm:^4.1.0": "patch:bn.js@npm%3A4.12.0#~/.yarn/patches/bn.js-npm-4.12.0-3ec6c884f6.patch", | ||
| "bn.js@npm:^4.0.0": "patch:bn.js@npm%3A4.12.0#~/.yarn/patches/bn.js-npm-4.12.0-3ec6c884f6.patch", | ||
| "bn.js@npm:^5.0.0": "patch:bn.js@npm%3A4.12.0#~/.yarn/patches/bn.js-npm-4.12.0-3ec6c884f6.patch", |
There was a problem hiding this comment.
When doing yarn from the host machine(not sure if I should have done it from the container), this is the error message I get. I'm not familiar with how patching works but I get the feeling that maybe locally generated patch files was not pushed to github from your machine. Please let me know if I need to do anything different to get this PR up and running in my machine. @Jovonni
anil@Anils-MacBook-Pro dapp-orchestration-basics % yarn
➤ YN0000: · Yarn 4.3.1
➤ YN0000: ┌ Project validation
➤ YN0057: │ dapp-agoric-orca-contract: Resolutions field will be ignored
➤ YN0000: └ Completed
➤ YN0000: ┌ Resolution step
➤ YN0001: │ Error: bn.js@patch:bn.js@npm%3A4.12.0#~/.yarn/patches/bn.js-npm-4.12.0-3ec6c884f6.patch: ENOENT: no such file or directory, open '/Users/anil/WebstormProjects/agoric-samples/dapp-orchestration-basics/.yarn/patches/bn.js-npm-4.12.0-3ec6c884f6.patch'
➤ YN0000: └ Completed in 0s 266ms
➤ YN0000: · Failed with errors in 0s 273ms
There was a problem hiding this comment.
@Jovonni let's find a way to drop these resolutions. As of Agoric/agoric-sdk#9691 there's no need to patch bn.js.
The change hasn't made it to a number release so you'll have to use dev build of @agoric/cosmic-proto@dev. I see your package.json does that already. If it's not working you may have to just bump which dev build is in the yarn.lock
There was a problem hiding this comment.
I just pushed the patch files , @amessbee should fix it for now:
https://github.com/Agoric/dapp-orchestration-basics/tree/dev/orca-v1/.yarn/patches
also, @turadg I can now try it without the patch files 🚀
| @@ -10,3 +10,5 @@ fund: | |||
| yarn workspace dapp-agoric-orca-contract fund | |||
| add-address: | |||
| yarn workspace dapp-agoric-orca-contract add:address | |||
| lint: | |||
There was a problem hiding this comment.
why is there a large yarn.lock diff in this commit?
…to the localaccount using the watcher pattern, and then transfers to remoteaccount upon successful deposit. Contract test also works with a deposit. also created transfer watcher
|
6ace05f updates:
The following balances are reached after submitting one offer with a Temporarily the address shows 1 
|
contract/src/orca.contract.js
Outdated
| const watcher = zone.exo( | ||
| `watcher-transfer-${localAddress.value}-to-${remoteAddress.value}`, // Error: key (a string) has already been used in this zone and incarnation -- perhaps use timestamp or offerid as well? | ||
| M.interface('watcher for transfer', { | ||
| onFulfilled: M.call(M.any()).optional(M.any()).returns(VowShape), | ||
| } | ||
| ), | ||
| { | ||
| /** | ||
| * @param {any} _result | ||
| * @param {bigint} value | ||
| */ | ||
| onFulfilled( | ||
| _result, | ||
| value | ||
| ) { | ||
| trace("inside onFulfilled:", value) | ||
| return watch(localAccount.transfer( | ||
| { | ||
| denom: "ubld", | ||
| value: value/2n, | ||
| }, | ||
| remoteAddress | ||
| )) | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| trace("about to watch transfer, watcher v0.16") | ||
| trace("watcher", watcher) | ||
| watch( | ||
| E(localAccount).deposit(pmt), | ||
| watcher, | ||
| BigInt(amt.value), | ||
| ); |
There was a problem hiding this comment.
| const watcher = zone.exo( | |
| `watcher-transfer-${localAddress.value}-to-${remoteAddress.value}`, // Error: key (a string) has already been used in this zone and incarnation -- perhaps use timestamp or offerid as well? | |
| M.interface('watcher for transfer', { | |
| onFulfilled: M.call(M.any()).optional(M.any()).returns(VowShape), | |
| } | |
| ), | |
| { | |
| /** | |
| * @param {any} _result | |
| * @param {bigint} value | |
| */ | |
| onFulfilled( | |
| _result, | |
| value | |
| ) { | |
| trace("inside onFulfilled:", value) | |
| return watch(localAccount.transfer( | |
| { | |
| denom: "ubld", | |
| value: value/2n, | |
| }, | |
| remoteAddress | |
| )) | |
| }, | |
| }, | |
| ); | |
| trace("about to watch transfer, watcher v0.16") | |
| trace("watcher", watcher) | |
| watch( | |
| E(localAccount).deposit(pmt), | |
| watcher, | |
| BigInt(amt.value), | |
| ); | |
| await E(localAccount).deposit(pmt); | |
| await localAccount.transfer( | |
| { | |
| denom: "ubld", | |
| value: amt.value / 2n, | |
| }, | |
| remoteAddress, | |
| ); |
Since we're inside an async-flow, we don't need to use a vows / a promise watcher.
Also - curious - why divide by 2?
There was a problem hiding this comment.
okay that makes sense. I'm just dividing by two as a demo/debug to show two accounts being funded, because if we transfer the entire amount in the end, sometimes it's not easily seen that localaccount ever had funds on the frontend. Just a debug thing for now
There was a problem hiding this comment.
I remember trying this first actually, and I kept getting this error here:
#11 (comment)
There was a problem hiding this comment.
Ah, I see what you mean here, its inside of vowTools.retriable, not just the handler! good point
There was a problem hiding this comment.
still encountering this when trying to do that, opened this ticket to track it @0xpatrickdev :
https://github.com/Agoric/dapp-orchestration-basics/issues/13#issue-2464084230
cleaned branch with all modern changes/updates
Tests Pass
Details
Contract deployment works as expected, and ui user flow works as expected.