Skip to content
This repository has been archived by the owner on Jan 4, 2023. It is now read-only.

Replace hashed input UtxoId with pair of (txID,outputIndex) #53

Merged
merged 12 commits into from
Jan 11, 2022
Merged

Conversation

rakita
Copy link
Contributor

@rakita rakita commented Dec 22, 2021

Change related to spec: FuelLabs/fuel-specs#253

Additionally i run clippy and fixed its warning. And there was one error that clippy gave:

error: written amount is not handled. Use `Write::write_all` instead
   --> src/receipt.rs:866:9
    |
866 |         instance.write(bytes)?;
    |         ^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[deny(clippy::unused_io_amount)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_io_amount

is_empty and `unwrap_or_else are clippy requests (she is very needy).

When this is merged i will go through other projects and fix if there is something broken.

@rakita rakita self-assigned this Dec 22, 2021
src/transaction.rs Outdated Show resolved Hide resolved
@rakita rakita requested a review from Voxelot December 23, 2021 10:46
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

lgtm, but will defer to @Voxelot and @vlopes11

@rakita
Copy link
Contributor Author

rakita commented Dec 30, 2021

@Voxelot @vlopes11 this is good to go and ready for review

@Voxelot
Copy link
Member

Voxelot commented Jan 4, 2022

I'd like to see a feature branch on fuel-vm and fuel-core with these changes working before merging. That way we don't wind up incrementally publishing fuel-tx as we troubleshoot.

@rakita rakita changed the title Replace hashes input UtxoId with pair of (txID,outputIndex) Replace hashed input UtxoId with pair of (txID,outputIndex) Jan 5, 2022
@rakita
Copy link
Contributor Author

rakita commented Jan 5, 2022

feature branch on fuel-vm and fuel-core with these changes working before merging. That way we don't wind up incrementally publishing fuel-tx as we troubleshoot.

Added a few more things to UtxoId (Copy,LowerHex,UpperHex,FromStr), and integrated new UtxoId into fuel-core here: https://github.com/FuelLabs/fuel-core/tree/utxo_id

Copy link
Contributor

@vlopes11 vlopes11 left a comment

Choose a reason for hiding this comment

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

Couple of nits/discussions

src/receipt.rs Outdated Show resolved Hide resolved
src/receipt.rs Outdated Show resolved Hide resolved
src/transaction.rs Show resolved Hide resolved
src/transaction/id.rs Show resolved Hide resolved
src/transaction/types/input.rs Outdated Show resolved Hide resolved
src/transaction/types/utxo_id.rs Outdated Show resolved Hide resolved
src/transaction/types/utxo_id.rs Outdated Show resolved Hide resolved
tests/bytes.rs Outdated Show resolved Hide resolved
adlerjohn
adlerjohn previously approved these changes Jan 9, 2022
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

LGTM, deferring to one of @Voxelot or @vlopes11 for 🦀 review

Voxelot
Voxelot previously approved these changes Jan 10, 2022
@rakita rakita requested a review from vlopes11 January 11, 2022 09:35
@rakita rakita dismissed stale reviews from Voxelot and adlerjohn via e8d1c57 January 11, 2022 09:37
@rakita rakita dismissed vlopes11’s stale review January 11, 2022 09:40

He is on vacation and all nitpicks were addressed in proper maner and approved by Brandon and John.

@rakita
Copy link
Contributor Author

rakita commented Jan 11, 2022

@adlerjohn @Voxelot i will need you to approve this once again, nothing has changed.

@rakita rakita merged commit 9e084f5 into master Jan 11, 2022
@rakita rakita deleted the utxo_id branch January 11, 2022 18:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants