This repository has been archived by the owner on Apr 15, 2024. It is now read-only.
Various cleanups and niceties #150
Merged
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
added 17 commits
September 9, 2021 11:48
These modules are deprecated and they introduce noise with outdated benches and tests
`assert!()` is the idiomatic way to do conditional panics in Rust. Aliasing it is actually confusing.
The `core::panic_if_not!()` macro is a transparent proxy to `std::assert!()` with some optional debugging log. Therefore, it's not unreasonable to shadow the `std::assert!()` macro instead of introducing a new name.
Some of the `PhantomData<F>` used throughtout the project are debatable. They feel like necessary because type arguments and bounds are placed too early in `impl` blocks instead of the very same methods that need them. However, type arguments in data types produce a cascade effect that infects everything the type passes by, introducing a significant amount of complexity, unreadability and, in some cases, actual contention to achieve certain things. One of the more evident cases is `BinaryBody`, which from a conceptual point of view shouldn't ever concern with the `PRP` except for when being wrapped or unwrapped, as we already have `WrappedMessage` and `UnwrappedMessage` that act as bundles of a message and a spongos state.
…entifier` Also add `Identifier::as_bytes()` to get a borrowed view of the Byte array, besides the owned `Identifier::to_bytes()`
…assert!()` (prev `panic_if_not!()`
These include: - `as_str(&self) -> Option<&str>` - `into_string(self) -> Option<String>` - `From<Vec<u8>> for Bytes` - `From<Bytes> for Vec<u8>` - `From<&[u8]> for Bytes` - `From<[u8; N]> for Bytes` - `From<&[u8; N]> for Bytes` - `AsRef<[u8]> for Bytes`
21 tasks
kwek20
reviewed
Oct 14, 2021
arnauorriols
changed the base branch from
support-selecting-transport-in-examples-with-env
to
develop
added 3 commits
October 30, 2021 00:45
…ak` benches `core-keccak` benches don't compile, and it is being migrated to crypto.rs, so there's no point in trying to make them work. `app-channels-example` is an example binary that should have its own environment much like `/examples`, specially since it needs nightly and `no-std` to compile.
Commit 655d28c fixed `User::reset_state()` by resetting also the link-store of the user. This is considered the proper behaviour of `User::reset_state()`, as a the common expectancy of a reseted user is for it to be as blank as possible, keeping only its identity data. This change, makes resetted users lose any data of the channel whatsoever, including subscription messages that cannot be recovered by fetching from the Tangle. This makes it a method only to be used in specific scenarios where for security reasons a user needs to trully be reseted; it also renders it not suitable as a means to _just_ replay the channel messages from the announcement message. The examples fixed in this commit were exemplifying the unsubscribe feature after `User::reset_state()` was called, and broke after the explained change for the aforementioned reason.
DyrellC
reviewed
Nov 1, 2021
arnauorriols
force-pushed
the
remove-phantomdata-from-binarybody-et-al
branch
from
74f5ed1 to
3384221
Compare
kwek20
approved these changes
Nov 3, 2021
kwek20
approved these changes
Jan 13, 2022
|
@DyrellC @kwek20 What do we want to do with chrono? General consensus is to migrate to time-rs, but I still think we should just remove the timestamp and get rid of the dependency altogether. And if we need/want to properly introduce a timestamp in the Streams messages, we'll deal with the issue then, which whichever crate is best then If you agree, I can remove just the timestamp and the dependency in this PR so that we fix the builds and remove the vulnerability warning, and finish the cleanup in #138 In any case, we need to merge this PR ASAP, to fix the build of the library. |
|
@arnauorriols I'm of the same opinion, we don't use the timestamp right now for anything and if it's determined that that will be valid to use going forward we can deal with it then. For now we should just remove the dependency and get this bad boy merged. |
added 4 commits
January 27, 2022 13:09
Bug was actually in the `try_or` macro of the error_handler module. Macros work strictly at syntactic level. Therefore, expressions passed to a macro are passed literally, before they are evaluated. It's necessary, if one wants to mimic the function-like semantics for the macro invocation, to take extra care not to expand the $expr variable more than once, to avoid triggering its side-effects more than once (unless that's explicitly intended).
arnauorriols
force-pushed
the
remove-phantomdata-from-binarybody-et-al
branch
2 times, most recently
from
70e49d5 to
7fcb6eb
Compare
|
Beautiful looking green checkmark right there. Let's get the ball rolling! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description of change
This PR is a miscellanea of small changes, cleanups and simplifications in preparation for the next PR. The most important changes included are:
PhantomData<F>from BinaryBodypanic_if_not!()toassert!()BytesIdentifierCursor<Link>This PR depends on #146
Type of change
How the change has been tested
Change checklist