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: pre-release mega cleanup #35

Merged
merged 10 commits into from
May 15, 2023
Merged

chore: pre-release mega cleanup #35

merged 10 commits into from
May 15, 2023

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented May 15, 2023

Main changes (which I can remember):

  • #[inline] where it makes sense
  • clean up some docs; write sol! docs and doctests, tested in ethers-abi-enc
  • make encode take &T, like Encoder::* methods, which allows removing Clone bound and calls
  • use more const-hex methods, including in fmt impls for fixed types
  • add crates/abi/src/coder/impl_core.rs for no alloc FixedArray encoding -- this is modified from bincode and libcore
  • set msrv to 1.65 for now
  • rename AbiResult to Result
  • replace TokenSeq's fn can_be_params() -> bool with const IS_TUPLE: bool
  • encode_with_selector -> encode to mimic decode_raw, decode.
  • format serde_json::json tests because VSCode auto detects the file as 2 line indent
  • optimize Address checksumming by not heap allocating at all
  • auto "solidity" -> "Solidity" (search and replace)
  • auto inline 1 "where" bounds into def: ea1998d
  • auto add a period to docs: 2de099a

Things I discovered:

  • Signed::to_{be,le}_bytes is not generic over N, when it should be
  • I think encode_packed_to does not produce the expected result with integers that use Signed or U256, so I made the comment a TODO. Packed encoding is just not tested in general.

replace
`fn (\w+)<\w+>(.*)\n\s*where\n\s*(.*),\n\s*\{`

with
`fn $1<$3>$2 {`
script:

```rust
fn main() {
    for file in std::env::args().skip(1) {
        let s = std::fs::read_to_string(&file).unwrap();
        let mut lines = s.lines().map(ToString::to_string).collect::<Vec<_>>();
        let len = lines.len().saturating_sub(1);
        for i in 0..len {
            let a = lines[i].trim();
            let b = lines[i + 1].trim();
            if a.starts_with("///")
                && !b.starts_with("///")
                && !a.ends_with('.')
                && !a.ends_with('>')
                && !a.ends_with('`')
                && !a.starts_with("/// [")
                && !a.ends_with("///")
            {
                lines[i].push('.');
            }
        }
        let mut lines: String = lines.join("\n");
        if s.ends_with('\n') {
            lines.push('\n');
        }
        std::fs::write(file, lines).unwrap();
    }
}
```
@DaniPopes DaniPopes requested a review from prestwich May 15, 2023 09:05
I assume this was inherited from Parity crates, but it is generally a bad
practice.

This commit simplifies `no_std_prelude` imports. Since `alloc` does not have to
be gated on the `std` feature, we can strip down the module to the basic `std`
prelude items.
crates/abi/src/types/mod.rs Outdated Show resolved Hide resolved
crates/abi/src/types/struct.rs Outdated Show resolved Hide resolved
crates/abi/src/types/struct.rs Show resolved Hide resolved
@@ -44,41 +42,36 @@ use crate::{no_std_prelude::*, token::TokenSeq, AbiResult, TokenType, Word};
/// const MY_STRUCT: MyStruct = MyStruct { a: true, b: [0x01, 0x02] };
/// ```
pub trait SolType {
/// The corresponding Rust type. This type may be borrowed (e.g. `str`)
/// The corresponding Rust type.
Copy link
Member

Choose a reason for hiding this comment

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

I tried hard to make that borrow work and could not get it ergonomically

crates/abi/src/types/udt.rs Show resolved Hide resolved
crates/abi/tests/doc_structs.rs Show resolved Hide resolved
crates/abi/tests/doc_function_like.rs Show resolved Hide resolved
crates/primitives/src/bits/fixed.rs Show resolved Hide resolved
crates/primitives/src/utils.rs Show resolved Hide resolved
/// Structs and enums:
///
/// ```ignore.
#[doc = include_str!("../../abi/tests/doc_structs.rs")]
Copy link
Member

Choose a reason for hiding this comment

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

love it

@DaniPopes DaniPopes requested a review from prestwich May 15, 2023 19:58
@DaniPopes DaniPopes merged commit 30dbda3 into main May 15, 2023
@DaniPopes DaniPopes deleted the dani/cleanup branch May 15, 2023 20:38
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