feat!: new encoding scheme for logs and encode RawSlice as u8
#1673
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
danielbate
requested review from
digorithm,
arboleya,
Torres-ssf,
Dhaiwat10 and
nedsalk
as code owners
…/feat/new-log-encoding
…/feat/new-log-encoding
Torres-ssf
reviewed
Feb 6, 2024
|
Coverage Report:
Changed Files:
|
|
Oh shoot, I didn't have the chance to review it, and it's merged already. 😅 I trust the reviewers. 🤞 |
|
Sorry @arboleya, I enabled auto merge because of the 2 SDK reviews, but also on the basis that a FE review would be the third reviewer. However it doesn't seem to have blocked? It has merged with 2 failing steps (both the FE review steps).
|
|
Any concerns can also be picked up in #1672 |
|
Yeah, it shouldn't be possible to merge this one without the review from someone in the FE team 🤔 . |
|
@danielbate Perhaps we should avoid using auto-merge in considerably big PRs to gather the maximum reviews possible. @Torres-ssf Yes, I will take a look at it later on. Maybe I need to change something in the repo settings, but I'm unsure. |
|
I believe in order for that to happen the 'FE conditional review' check should be required if it is enabled. |
|
@Dhaiwat10 I believe that was the problem. The check didn't pass but the PR got merged anyway. |
|
@Torres-ssf Yes. That's what I'll double check in the repo settings. |
6 tasks
This was referenced Jun 20, 2024
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.
Closes #1671
This PR adds support for sway's experimental encoding in logs. This is demonstrated via a new test suite, which is a subset of
fuel-gauge, which builds anotherforcworkspace using the experimental flag (--experimental-new-encoding).There are opportunities for refactoring here, however that shall be detailed and implemented as part of #1672 given this PR is already over 2K lines.
Breaking Change
RawSlicedata encoded as unpaddedu8, also implemented by the RS SDK. Currently we are encoding it asu64and doing additional arithmetic onencodeanddecodeto compute the byte length. ARawSliceindicates a slice of bytes so we should treat it as so.This poses a problem either on
v0orv1, one will have to compromise to the other asv1assumes the data asu8so we'd have to do additional arithmetic to get the correctlen, rather than decoding it from the byte data. I have opted to introduce the breaking change inv0as it brings us closer tov1.Type Support
u8u16u32u64b256b512str[n])struct String)Further changes
ABIEncodeimplementations fromsway-std-libv1codersdecodevalidation #1426Blockers
0.49.2upgrade - chore: upgradingforcto0.49.2#1707