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

Add an action to update assumed valid target periodically #211

Closed
Keith-CY opened this issue Jul 6, 2023 · 10 comments
Closed

Add an action to update assumed valid target periodically #211

Keith-CY opened this issue Jul 6, 2023 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@Keith-CY
Copy link
Member

Keith-CY commented Jul 6, 2023

The CKB_NODE_ASSUME_VALID_TARGET defined at https://github.com/nervosnetwork/neuron/blob/develop/packages/neuron-wallet/.env#L120 is used on launching the built-in CKB node to boost performance.

It's expected to be at least 7 days before the release day and was updated manually. It introduced a mental burden in the release workflow, so it's expected to be updated automatically by the CI.

The following work will be done:

  1. add a script to fetch the hash of a block minted 1 day ago;
  2. add a CI to run the script when a PR is opened from rc/* to master, and the update should open a new PR to the rc/*

It may take a long time to merge rc/* into master so that assumed valid target is more than 7 days before the release day, but it's acceptable IMO.

Or the script could be run on merge from rc/* to master and the commit is adopted on the main while the script sets 7 days before rather than 1 day before.

@Keith-CY Keith-CY added the enhancement New feature or request label Jul 6, 2023
@Keith-CY Keith-CY added this to Neuron Jul 6, 2023
@Keith-CY Keith-CY self-assigned this Jul 6, 2023
@Danie0918 Danie0918 moved this to 🆕 New in Neuron Jul 6, 2023
@Danie0918 Danie0918 assigned yanguoyu and unassigned Keith-CY Jul 10, 2023
@Danie0918 Danie0918 moved this from 🆕 New to 🏗 In Progress in Neuron Jul 10, 2023
@yanguoyu
Copy link

  1. Write a shell command to update CKB_NODE_ASSUME_VALID_TARGET.
    1. Get the tip header block number, and get the block hash with (the header block number - 10000)
    2. Read the env file and update the CKB_NODE_ASSUME_VALID_TARGET
  2. create an action to create PR to the rc/* branch
    1. trigger event: pull_request opened
    2. use https://github.com/technote-space/create-pr-action to create a PR to update the assume valid target

@Keith-CY
Copy link
Member Author

  1. Get the tip header block number, and get the block hash with (the header block number - 10000)

IMG, the rc-branch should be fully tested for a week, so the tip block hash could be set when the PR is open

create an action to create PR to the rc/* branch

Does it mean trigger an action run when the PR from rc/* to main is opened

Write a shell command

Personally a JavaScript script is preferred because it's easy for reviewers and maintainers, and JavaScript is supported in GitHub runner with https://github.com/actions/github-script

@yanguoyu
Copy link

Does it mean trigger an action run when the PR from rc/* to main is opened

yes, but I believe the trigger event can be set as ready-for-review currently. After the pr is ready for review, it will merge soon.

Personally a JavaScript script is preferred because it's easy for reviewers and maintainers, and JavaScript is supported in GitHub runner with https://github.com/actions/github-script

Javascript is also no problem, I will wrap the process into a npm command. Then we only need to npm run update:valid:target to change the local env file.

@Keith-CY
Copy link
Member Author

Does it mean trigger an action run when the PR from rc/* to main is opened

yes, but I believe the trigger event can be set as ready-for-review currently. After the pr is ready for review, it will merge soon.

Personally a JavaScript script is preferred because it's easy for reviewers and maintainers, and JavaScript is supported in GitHub runner with actions/github-script

Javascript is also no problem, I will wrap the process into a npm command. Then we only need to npm run update:valid:target to change the local env file.

Triggered by ready-for-review is a good idea

@yanguoyu
Copy link

yanguoyu commented Jul 17, 2023

https://github.com/technote-space/create-pr-action not support ready-for-review event. So I will use other actions to achieve it.

  • Change the local file.
  • Add and commit
  • Create PR

I have found some action plugins to add and commit, but because of actions/runner#667
the commit could not be verified. So there are two ways to avoid this.

  1. Refer to https://gist.github.com/swinton/03e84635b45c78353b1f71e41007fc7c, and we can use github script
  2. Use crazy-max/ghaction-import-gpg@v2 to verify the commit, but it needs us to set the GPG to the environment.

Cause of we currently only update one file, I would prefer the first resolution.

@yanguoyu
Copy link

@Danie0918 Danie0918 moved this from 🏗 In Progress to 🔎 Code Review in Neuron Jul 24, 2023
@Danie0918 Danie0918 moved this from 🔎 Code Review to ✅ Done in Neuron Jul 24, 2023
@Keith-CY Keith-CY reopened this Sep 20, 2023
@Keith-CY
Copy link
Member Author

The timing to update assume valid target is set ready for review on master branch, but changelog is expected to be updated at rc/v* branch. The assume valid target will be included in changelog, so it's better to update assume valid target on rc/v* branch

@yanguoyu
Copy link

The timing to update assume valid target is set ready for review on master branch, but changelog is expected to be updated at rc/v* branch. The assume valid target will be included in changelog, so it's better to update assume valid target on rc/v* branch

How about when trigger on the create event or the push event on the rc/v* branch?

@Keith-CY
Copy link
Member Author

Keith-CY commented Oct 12, 2023

The timing to update assume valid target is set ready for review on master branch, but changelog is expected to be updated at rc/v* branch. The assume valid target will be included in changelog, so it's better to update assume valid target on rc/v* branch

How about when trigger on the create event or the push event on the rc/v* branch?

It should work to trigger this workflow when rc/v* branch is created

@yanguoyu
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

3 participants