Replace H160 in config and cli options of relayer by Bytes20 #2116

AurelienFT · 2024-08-20T15:35:42Z

Linked Issues/PRs

Description

Replace H160 in config and cli options of relayer by Bytes20. This might cause a bit of performance impact as it's needed to transform them to H160 to interact with ethers_core dependency.

(This PR follows #1951)

Checklist

Breaking changes are clearly marked as such in the PR description and changelog
New behavior is reflected in tests
The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

I have reviewed the code myself
I have created follow-up issues caused by this PR and linked them here

Voxelot · 2024-08-20T18:48:58Z

We should double check if this will break the integ tests in the rust sdk since it is a change to a public interface at the library level. If so let's make a ticket on fuels-rs so they are aware of this incoming change.

AurelienFT · 2024-08-21T08:50:36Z

Ok, I'm in the process of doing it. (blocked by : FuelLabs/fuels-rs#1492 for now)

AurelienFT · 2024-08-22T12:42:38Z

We should double check if this will break the integ tests in the rust sdk since it is a change to a public interface at the library level. If so let's make a ticket on fuels-rs so they are aware of this incoming change.

I ran the tests of fuels-rs and didn't had any fail.

xgreenx · 2024-08-22T12:47:22Z

bin/fuel-core/src/cli/run/relayer.rs

@@ -53,10 +50,6 @@ pub struct RelayerArgs {
    pub syncing_log_frequency_secs: u64,
 }

-pub fn parse_h160(input: &str) -> Result<H160, <H160 as FromStr>::Err> {


Would be nice to see a test that verifies the decoded value

The function on which you added your comment has been removed. Can you clarify what you want to be tested ?

It would be nice to test that parsing of the H160::from_str and Bytes20::from_str returns the same values=)

netrome

LGTM, but I think it would be nice if we could use the ethers_core::Address type alias in the remaining places you reference the H160 type.

crates/services/relayer/src/service/get_logs/test.rs

netrome

Looks good to me, just one more place to change H160 to ethereum_types::Address

tests/tests/relayer.rs

netrome

Nice!

CHANGELOG.md

xgreenx · 2024-08-26T16:43:59Z

bin/fuel-core/src/cli/run/relayer.rs

@@ -53,10 +50,6 @@ pub struct RelayerArgs {
    pub syncing_log_frequency_secs: u64,
 }

-pub fn parse_h160(input: &str) -> Result<H160, <H160 as FromStr>::Err> {


It would be nice to test that parsing of the H160::from_str and Bytes20::from_str returns the same values=)

xgreenx · 2024-08-26T16:47:14Z

tests/Cargo.toml

@@ -25,6 +25,7 @@ aws-config = { version = "1.1.7", features = [
 aws-sdk-kms = { version = "1.37.0", optional = true }
 clap = { workspace = true }
 cynic = { workspace = true }
+ethereum-types = "0.14.1"


It would be nice if relayer service exported this dependency instead. Maybe behind the test-helpers feature. In this case we don't need to include it here.

Also relayer service could provide the covnert_to_address function, and we could use it instead of the Address type.

Oops, didn't see, seems like you did it :D

@AurelienFT

## 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

AurelienFT added 2 commits August 20, 2024 17:34

Replace H160 in config and cli options of relayer by Bytes20

426e26f

Update CHANGELOG.md

e4b4de5

AurelienFT requested a review from a team August 22, 2024 12:42

xgreenx reviewed Aug 22, 2024

View reviewed changes

netrome previously approved these changes Aug 22, 2024

View reviewed changes

crates/services/relayer/src/service/get_logs/test.rs Outdated Show resolved Hide resolved

AurelienFT self-assigned this Aug 26, 2024

AurelienFT and others added 2 commits August 26, 2024 09:18

Merge branch 'master' into use_bytes20_relayer

73b46bb

Use alias type ethereum_types::Address in place of H160

3fff50f

AurelienFT dismissed netrome’s stale review via 3fff50f August 26, 2024 07:26

AurelienFT requested review from xgreenx and netrome August 26, 2024 07:26

netrome previously approved these changes Aug 26, 2024

View reviewed changes

tests/tests/relayer.rs Outdated Show resolved Hide resolved

Use alias type ethereum_types::Address in place of H160

4397d1a

AurelienFT dismissed netrome’s stale review via 4397d1a August 26, 2024 07:49

AurelienFT requested a review from netrome August 26, 2024 07:49

netrome previously approved these changes Aug 26, 2024

View reviewed changes

Merge branch 'master' into use_bytes20_relayer

472e615

xgreenx reviewed Aug 26, 2024

View reviewed changes

AurelienFT and others added 2 commits August 27, 2024 09:20

Merge branch 'master' into use_bytes20_relayer

9196c2f

Add test to verify backward compatibility

6676c8b

AurelienFT dismissed netrome’s stale review via 6676c8b August 27, 2024 07:31

AurelienFT requested review from xgreenx and netrome August 27, 2024 07:32

AurelienFT and others added 2 commits August 27, 2024 10:44

Merge branch 'master' into use_bytes20_relayer

084ca02

Remove ethereum-types dependency form integration tests

1dc5018

xgreenx approved these changes Aug 27, 2024

View reviewed changes

Merge branch 'master' into use_bytes20_relayer

dc4fb88

netrome approved these changes Aug 27, 2024

View reviewed changes

xgreenx merged commit 2f1c3b7 into master Aug 27, 2024
34 of 35 checks passed

xgreenx deleted the use_bytes20_relayer branch August 27, 2024 11:04

xgreenx mentioned this pull request Aug 28, 2024

Release v0.35.0 #2136

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace H160 in config and cli options of relayer by Bytes20 #2116

Replace H160 in config and cli options of relayer by Bytes20 #2116

AurelienFT commented Aug 20, 2024 •

edited

Loading

Voxelot commented Aug 20, 2024

AurelienFT commented Aug 21, 2024

AurelienFT commented Aug 22, 2024

xgreenx Aug 22, 2024

AurelienFT Aug 26, 2024

xgreenx Aug 26, 2024

AurelienFT Aug 27, 2024

netrome left a comment

netrome left a comment

netrome left a comment

xgreenx Aug 26, 2024

xgreenx Aug 26, 2024

AurelienFT Aug 27, 2024

Replace H160 in config and cli options of relayer by Bytes20 #2116

Replace H160 in config and cli options of relayer by Bytes20 #2116

Conversation

AurelienFT commented Aug 20, 2024 • edited Loading

Linked Issues/PRs

Description

Checklist

Before requesting review

Voxelot commented Aug 20, 2024

AurelienFT commented Aug 21, 2024

AurelienFT commented Aug 22, 2024

xgreenx Aug 22, 2024

Choose a reason for hiding this comment

AurelienFT Aug 26, 2024

Choose a reason for hiding this comment

xgreenx Aug 26, 2024

Choose a reason for hiding this comment

AurelienFT Aug 27, 2024

Choose a reason for hiding this comment

netrome left a comment

Choose a reason for hiding this comment

netrome left a comment

Choose a reason for hiding this comment

netrome left a comment

Choose a reason for hiding this comment

xgreenx Aug 26, 2024

Choose a reason for hiding this comment

xgreenx Aug 26, 2024

Choose a reason for hiding this comment

AurelienFT Aug 27, 2024

Choose a reason for hiding this comment

AurelienFT commented Aug 20, 2024 •

edited

Loading