Skip to content

chore: move ethers utils to its own package#35

Closed
cbrzn wants to merge 4 commits into
devfrom
cbrzn/ethers-utils
Closed

chore: move ethers utils to its own package#35
cbrzn wants to merge 4 commits into
devfrom
cbrzn/ethers-utils

Conversation

@cbrzn
Copy link
Copy Markdown
Contributor

@cbrzn cbrzn commented Mar 2, 2023

this PR aims to create the ethers-utils wrapper so it can be used in the safe-wrapper

the long-term of this is to do a cargo workspace with the different ethereum wrappers (core, utils, signer, transaction)

Copy link
Copy Markdown
Contributor

@krisbitney krisbitney left a comment

Choose a reason for hiding this comment

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

Looks great!

Could you add CI?

@@ -0,0 +1,15 @@
type Module {
solidityKeccak256Bytes(bytes: String!): String!
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This method has bytes in the name but accepts a string argument and returns a string. Maybe adjust the name? Same with other methods

Comment thread wrapper/src/lib.rs
pub fn get_signer_address(args: wrap::ArgsGetSignerAddress) -> String {
let address = PolywrapSigner::new(&args.connection).address();
format!("{:#x}", address)
format!("{:#x}", address).to_string().to_lowercase()
Copy link
Copy Markdown
Contributor

@krisbitney krisbitney Mar 3, 2023

Choose a reason for hiding this comment

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

format!("{:#x}", address) already returns a lowercase String

@cbrzn
Copy link
Copy Markdown
Contributor Author

cbrzn commented Mar 10, 2023

converting this to draft. this is just for example purposes, and its being used by the safe wrapper.

after talking with @krisbitney we're going to take the path of improving the logic in PR #41. once the PR 41 has been completed we can close this

@cbrzn cbrzn marked this pull request as draft March 10, 2023 18:19
@dOrgJelli dOrgJelli changed the base branch from main to dev March 13, 2023 21:53
@cbrzn cbrzn closed this Apr 18, 2023
@cbrzn cbrzn deleted the cbrzn/ethers-utils branch August 24, 2023 16:23
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.

2 participants