Secp256k1 RecoveryID correction for KMS-genrated signatures #2134
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dentosal
changed the title
Dentosal
changed the title
Dentosal
changed the title
rymnc
previously approved these changes
Aug 27, 2024
| - [2119](https://github.com/FuelLabs/fuel-core/pull/2119): GraphQL query fields for retrieving information about upgrades. | ||
| - [2116](https://github.com/FuelLabs/fuel-core/pull/2116): Replace `H160` in config and cli options of relayer by `Bytes20` of `fuel-types` | ||
| ### Fixed | ||
| - [2134](https://github.com/FuelLabs/fuel-core/pull/2134): Perform RecoveryID normalization for AWS KMS -generated signatures. |
There was a problem hiding this comment.
Suggested change
| - [2134](https://github.com/FuelLabs/fuel-core/pull/2134): Perform RecoveryID normalization for AWS KMS -generated signatures. | |
| - [2134](https://github.com/FuelLabs/fuel-core/pull/2134): Perform RecoveryID normalization for AWS KMS generated signatures. |
There was a problem hiding this comment.
Oh good point I ddn't realize the space-before-dash compound-noun-modifiers are nonstandard English. After a long consultation with ChatGPT and couple of style guides, I'm still convinced that the form I used clearner than the alternative without the dash. If you feel strongly about this, the sentence could be reformulated as
Perform RecoveryID normalization for signatures generated by AWS KMS.
netrome
previously approved these changes
Aug 28, 2024
netrome
approved these changes
Aug 28, 2024
rymnc
approved these changes
Aug 28, 2024
Merged
xgreenx
added a commit
that referenced
this pull request
Aug 28, 2024
## Version v0.35.0 ### Added - [2122](#2122): Changed the relayer URI address to be a vector and use a quorum provider. The `relayer` argument now supports multiple URLs to fetch information from different sources. - [2119](#2119): GraphQL query fields for retrieving information about upgrades. ### Changed - [2113](#2113): Modify the way the gas price service and shared algo is initialized to have some default value based on best guess instead of `None`, and initialize service before graphql. - [2112](#2112): Alter the way the sealed blocks are fetched with a given height. - [2120](#2120): Added `submitAndAwaitStatus` subscription endpoint which returns the `SubmittedStatus` after the transaction is submitted as well as the `TransactionStatus` subscription. - [2115](#2115): Add test for `SignMode` `is_available` method. - [2124](#2124): Generalize the way p2p req/res protocol handles requests. #### Breaking - [2040](#2040): Added full `no_std` support state transition related crates. The crates now require the "alloc" feature to be enabled. Following crates are affected: - `fuel-core-types` - `fuel-core-storage` - `fuel-core-executor` - [2116](#2116): Replace `H160` in config and cli options of relayer by `Bytes20` of `fuel-types` ### Fixed - [2134](#2134): Perform RecoveryID normalization for AWS KMS -generated signatures. ## What's Changed * Change TODO clippy to wait for false positive correction by @AurelienFT in #2110 * Remove option on shared algo, give default algo, reorder service startup by @MitchTurner in #2113 * chore(sealed_blocks): optimize fetching of sealed block header at a given height by @rymnc in #2112 * chore(p2p_service): remove unnecessary cast to usize by @rymnc in #2123 * Added a benchmark for the predicate with ed19 verification by @xgreenx in #2127 * Add test SignMode is_available by @AurelienFT in #2115 * feat: Add graphql query fields for retreiving information about upgrades by @netrome in #2119 * Small code optimizations by @MoneyBund in #2035 * Change relayer URI address to be vector and use quorum provider by @AurelienFT in #2122 * chore(p2p_service): clean up processing of p2p req/res protocol by @rymnc in #2124 * feat: add `submitAndAwaitStatus` subscription endpoint by @maschad in #2120 * Replace H160 in config and cli options of relayer by Bytes20 by @AurelienFT in #2116 * Secp256k1 RecoveryID correction for KMS-genrated signatures by @Dentosal in #2134 * Added `no_std` support state transition related crates by @xgreenx in #2040 ## New Contributors * @MoneyBund made their first contribution in #2035 * @maschad made their first contribution in #2120 **Full Changelog**: v0.34.0...v0.35.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
AWS KMS doesn't insert recovery ids into otherwise-recoverable secp256k1 signatures. This PR patches the process in the same way we already do in fuel-vm.
Checklist
Before requesting review