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

chore: improve guide #193

Merged
merged 27 commits into from
May 31, 2024
Merged

chore: improve guide #193

merged 27 commits into from
May 31, 2024

Conversation

RafilxTenfen
Copy link
Contributor

@RafilxTenfen RafilxTenfen commented May 27, 2024

Modifications for improvements on the guide

  • Add a reminder for json registry information in PR to be exactly the same as the one used to sign, not just the content
  • Update BTC staker to not use the example btc-pk in command, use <staker_pk> instead
  • Add an explicit section to discuss deposit amounts and amounts lower than 0.1sBTC will be considered invalid registration
  • Script to validate new finality provider information locally (runs the same checks as the CI will do on the PR)
  • Section to verify the signed staking transaction before sending to BTC
  • Add validation of commission 0.[0-9]+

@RafilxTenfen RafilxTenfen self-assigned this May 27, 2024
Copy link
Member

@vitsalis vitsalis left a comment

Choose a reason for hiding this comment

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

Nice!

bbn-test-4/finality-providers/README.md Show resolved Hide resolved
bbn-test-4/finality-providers/README.md Outdated Show resolved Hide resolved
bbn-test-4/finality-providers/README.md Outdated Show resolved Hide resolved
bbn-test-4/finality-providers/README.md Show resolved Hide resolved
bbn-test-4/finality-providers/verify-new-fp.sh Outdated Show resolved Hide resolved
@RafilxTenfen
Copy link
Contributor Author

Copy link
Member

@vitsalis vitsalis left a comment

Choose a reason for hiding this comment

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

lgtm, some minor comments

bbn-test-4/finality-providers/README.md Outdated Show resolved Hide resolved
bbn-test-4/finality-providers/README.md Outdated Show resolved Hide resolved
bbn-test-4/finality-providers/README.md Outdated Show resolved Hide resolved
bbn-test-4/finality-providers/README.md Outdated Show resolved Hide resolved
bbn-test-4/finality-providers/README.md Outdated Show resolved Hide resolved
bbn-test-4/finality-providers/scripts/fp-check-tx.sh Outdated Show resolved Hide resolved
exit 1
fi

FP_CHANGED_FILES=$(fpChanged)
Copy link
Member

Choose a reason for hiding this comment

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

Where does the fpChanged variable become set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is a function we are calling loaded from . $CWD/fp-changed.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RafilxTenfen RafilxTenfen merged commit a4db0ac into main May 31, 2024
3 checks passed
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.

Collect PR failures data on finality provider registration to improve guide
3 participants