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

upgrade-test: deploy KREAd contract #8361

Closed
wants to merge 15 commits into from
Closed

upgrade-test: deploy KREAd contract #8361

wants to merge 15 commits into from

Conversation

dckc
Copy link
Member

@dckc dckc commented Sep 21, 2023

closes: #8329
refs: #XXXX

various bits still todo:

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

@dckc
Copy link
Member Author

dckc commented Sep 21, 2023

Integrating governance looks like porting the kread-committee and start-kread targets from the Makefile in Kryha/KREAd#3

https://github.com/agoric-labs/KREAd/blob/09bf292cc4a92b6a3d015a87c159b7f4db94894e/agoric/Makefile#L43-L61

@dckc dckc force-pushed the dc-mn2-test branch 2 times, most recently from b5673a3 to f645c4e Compare September 22, 2023 02:20
@dckc
Copy link
Member Author

dckc commented Sep 22, 2023

@turadg , @Chris-Hibbert , It's not completely working (TODO: provision wallets), but I integrated the governance and agoric run style stuff:

mn2-start.test.js has a @file comment explaining how it's supposed to work.
currently: c5964fe

it depends on agoric-labs/KREAd#1

@turadg turadg force-pushed the dc-mn2-test branch 2 times, most recently from bf72780 to 7264d15 Compare September 26, 2023 14:56
@dckc dckc marked this pull request as ready for review October 5, 2023 16:48
@dckc dckc requested review from iomekam and turadg October 5, 2023 16:48
@dckc
Copy link
Member Author

dckc commented Oct 5, 2023

I wonder how much git history clean-up is worthwhile here...

@turadg
Copy link
Member

turadg commented Oct 5, 2023

I wonder how much git history clean-up is worthwhile here...

I could see this going into one squash once the changes are cleaned up. I think we want to:

  • drop the NO_BUILD thing since it doesn't work, and fix that separately
  • have the agops update happen in a COPY for the layer that needs it (can copy from master now that it's in there)
  • clean up superfluous docs changes (like newlines)

@Chris-Hibbert
Copy link
Contributor

I wonder how much git history clean-up is worthwhile here...

Maybe this is obvious, but with 46 commits on 29 files, I only see two plausible choices: (1) squash the whole thing, or (2) if there are two or three main features, then all the changes could be split into a few commits.

dckc and others added 11 commits October 5, 2023 20:51
 - add /mn2 mount to upgrade-test container
 - check swingstore before
 - succeed if bundle already installed
 - check tx result, vstorage after
 - mint IST if needed
 - assert / Fail shim
 - unmarshal w/o endo
   - for metrics / governance (not needed?)
 - refactor: clarify magnitude of debt limit
 - refactor: fix mintIST arg names
 - assert shim
 - refactor: factor out access to brands, instances; smallCaps fmt
   - smallCapsContext
   - wellKnownIdentities
 - refactor: export dbTool
 - borrow isStreamCell from @agoric/internal (WIP)
 - agd CLI utilities
   from 371182c 12 Jul
   https://github.com/Agoric/agoric-sdk/blob/6045ee17ce409400e48deb8e470f39b0a192b91e/packages/inter-cli/src/lib/agd-lib.js
 - chore: agd.keys.add API
 - CONTRACT INIT SUCCESS!
 - test: ensure bundles installed (from agoric run)
 - adapt core eval test to `agoric run` style
 - factor out testIncludes
 - code clean-up, docs
 - return proposal info once it passes
   - and only query once per block
use `exec -it` to get a shell
@dckc
Copy link
Member Author

dckc commented Oct 6, 2023

I reduced the 46 commits to 14 without changing any bytes in the results (351eb2a vs. 81201cc).

I think we want to:

  • drop the NO_BUILD thing since it doesn't work, and fix that separately

For now, I moved it last

2023-09-22 18:20 81201cc feat: agd NO_BUILD option

have the agops update happen in a COPY for the layer that needs it (can copy from master now that it's in there)

I'm not sure I understand that; might need help.

I moved this change to its own commit:

2023-10-05 21:02 14ab855 @@@ agops unversioned???

clean up superfluous docs changes (like newlines)

This change talks about tmux, but it's a bunch of README tweaks. I'm not sure if that's a result of me separating it from the actual tmux change or something...

2023-09-25 14:54 76116e2 test: remove tmux from upgrade-tests

@turadg
Copy link
Member

turadg commented Oct 6, 2023

2023-09-22 18:20 81201cc feat: agd NO_BUILD option

I've never seen it work. Does it work for you? Adding a broken option does more harm than good.

have the agops update happen in a COPY for the layer that needs it (can copy from master now that it's in there)
I'm not sure I understand that; might need help.

The README has a section, # Patch agoric-cli. Instead the Dockerfile should take care of that.

Happy to help tho

2023-10-05 21:02 14ab855 @@@ agops unversioned???

That commit should just be dropped. "docker" was just for the version that lived only in Docker. But now it comes from agoric-cli. (Where it's --version is "unversioned"; if you propose changing that we could but it's far from the scope of this PR)

2023-09-25 14:54 76116e2 test: remove tmux from upgrade-tests

Should be dropped. The tmux changes made it into master separately. #8393

@dckc
Copy link
Member Author

dckc commented Jan 19, 2024

@dckc dckc closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deployment upgrade test of Kread
3 participants