Skip to content

feat: add agoric-upgrade-19 (proposal 91) #228

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

Merged
merged 1 commit into from
Apr 10, 2025

Conversation

muhammadahmadasifbhatti
Copy link
Contributor

@muhammadahmadasifbhatti muhammadahmadasifbhatti commented Apr 4, 2025

Description

This PR adopts governance proposal 91 as per upgrade 19. Created via following instructions here.
Companion agoric-sdk PR

When a proposal passes on agoric-3 Mainnet, it should be included in the history that this synthetic image tracks.

  • before this PR, change any fromTag using latest (such as a3p-integration) to use a fixed version (otherwise they will fail when this PR changes latest and they pick it up)
  • before merging this PR, include a link to a PR that adopts its fromTag use-${proposalName} (where proposalName is the part of the agoric-3-proposals proposal directory name after the colon, cf. a3p-integration/proposals)
  • after this PR merges, merge that other PR

@Muneeb147
Copy link

Muneeb147 commented Apr 4, 2025

@muhammadahmadasifbhatti We need to keep feat: add agoric-upgrade-19 (proposal 91) as single feat commit instead of init (might need to create a separate PR with the right commit incase we can't update the existing commit's message).
Also can we look at failure step? What is it about? We might need help from Mathieu or Mujahid for that.

@mujahidkay
Copy link
Member

mujahidkay commented Apr 4, 2025

@muhammadahmadasifbhatti We need to keep feat: add agoric-upgrade-19 (proposal 91) as single feat commit instead of init (might need to create a separate PR with the right commit incase we can't update the existing commit's message).

Can just reword commit.

Also can we look at failure step? What is it about? We might need help from Mathieu or Mujahid for that.

@muhammadahmadasifbhatti we also need to copy the patches folder from a3p-integration

@Muneeb147
Copy link

Muneeb147 commented Apr 4, 2025

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This README needs to be updated. It usually contains info regarding when the proposal was adopted on the chain i.e. the block height and its respective block time (see previous proposals for ref)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@muhammadahmadasifbhatti muhammadahmadasifbhatti force-pushed the ahmad/migrate-u19 branch 3 times, most recently from 1eb13a6 to ee7dfc7 Compare April 7, 2025 08:48
@muhammadahmadasifbhatti
Copy link
Contributor Author

muhammadahmadasifbhatti commented Apr 7, 2025

@mhofman @turadg I have created this PR to adopt the governance proposal 91 as per the upgrade 19. Some tests are failing and I am unable to find the exact underlying reason.

I also checked why these tests were passing in the agoric-sdk on dev-upgrade-19 PR. It seemed like the last proposal layer was fast-usdc-beta instead of fast-usdc-rc1. So, I created this PR to see if the tests are passing by removing the fast-usdc-rc1 proposal. They are still failing due to the same reason.

Input will be appreciated.

@Muneeb147
Copy link

@muhammadahmadasifbhatti Can you please share more details? Which test exactly? What is the exact error? Is it consistently failing at one single point?

@muhammadahmadasifbhatti
Copy link
Contributor Author

@Muneeb147

Can you please share more details? Which test exactly?

I have attached the links to the failed tests

What is the exact error?

Exact error on the console is:

console.error(`Run of ${name} failed with exit code ${runResult.status}`);
                                         ^
ReferenceError: name is not defined

But this is not the cause. Seems like some directories are missing that should have been created when we ran script/buill-all-submissions.sh.

Is it consistently failing at one single point?

Yes, it is consitently failing at the same point.

@Muneeb147
Copy link

@muhammadahmadasifbhatti
So pasting the exact test/error in the comment helps as the viewer can directly see the root-cause rather than having a friction of jumping to the links and finding the error.

@Muneeb147
Copy link

@muhammadahmadasifbhatti

Error: axios@patch:axios@npm%3A1.7.7#~/.yarn/patches/axios-npm-1.7.7-cfbedc233d.patch: ENOENT: no such file or directory, open '/usr/src/proposals/91:upgrade-19/.yarn/patches/axios-npm-1.7.7-cfbedc233d.patch'

Why are we seeing this error again? I see you have added patches folder in recent commit. Landed on this error from your Some tests link.

@muhammadahmadasifbhatti
Copy link
Contributor Author

@Muneeb147 Wrong Link was attached. Updated it.

New update: The issue is with the scripts/build-all-submissions.sh. Ideally it should create the directories given in the sdk-generate in the package.json but, that's not the case.

I re-ran the scripts/build-all-submissions.sh and got this error.

#  node[89177]: void node::fs::InternalModuleStat(const FunctionCallbackInfo<Value> &) at ../src/node_file.cc:1056
  #  Assertion failed: (args.Length()) >= (2)

----- Native stack trace -----

 1: 0x102350840 node::Assert(node::AssertionInfo const&) [/opt/homebrew/Cellar/node/23.11.0/bin/node]
 2: 0x10236507c node::fs::InternalModuleStat(v8::FunctionCallbackInfo<v8::Value> const&) [/opt/homebrew/Cellar/node/23.11.0/bin/node]
 ...

Looking further into it.

@Muneeb147
Copy link

@muhammadahmadasifbhatti I see all green. That's great Ahmad ❤️❤️
So how did you figure out the problem and the solution eventually? Do we need to update docs as well for future reference?

Copy link
Member

@mujahidkay mujahidkay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please mark the relevant checkboxes as complete, and the remaining once you open the companion PR.

@muhammadahmadasifbhatti
Copy link
Contributor Author

@muhammadahmadasifbhatti I see all green. That's great Ahmad ❤️❤️ So how did you figure out the problem and the solution eventually? Do we need to update docs as well for future reference?

I was not able to generate the directories mentioned in the sdk-generate in package.json due to a node error. Actually, I changed my machine recently, and it was using the latest node version. I checked the agorik-sdk docs and they state

We generally support latest version

So, I changed the version to the minimum version supported by agorik-sdk and got the sdk-generate directories.

@Chris-Hibbert
Copy link
Contributor

I'm currently working on adding the fastUSDC coreEvals (proposals 87 and 88) to agoric-3-proposals. Please don't remove the current contents of n:upgrade-next and g:gtm-fast-usdc from a3p-integration until we have all these migrated.

@muhammadahmadasifbhatti muhammadahmadasifbhatti merged commit 9c72c29 into main Apr 10, 2025
2 checks passed
@muhammadahmadasifbhatti muhammadahmadasifbhatti deleted the ahmad/migrate-u19 branch April 10, 2025 10:22
mergify bot added a commit to Agoric/agoric-sdk that referenced this pull request Apr 14, 2025
Agoric/agoric-3-proposals#228

## Description
This PR updates a3p to use upgrade-19 image.

### Security Considerations
None

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
None

### Upgrade Considerations
None
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.

4 participants