8000 Rename `TxOutRef` to `OutPoint` and use it in `TxIn`. by jeandudey · Pull Request #139 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Rename TxOutRef to OutPoint and use it in TxIn. #139

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

Merged
merged 1 commit into from
Aug 20, 2018

Conversation

jeandudey
Copy link
Contributor

No description provided.

@jeandudey jeandudey force-pushed the 2018-08-18-outpoint branch 2 times, most recently from b17d2d2 to 92a2644 Compare August 18, 2018 15:36
@apoelstra
Copy link
Member

cc @TheBlueMatt concept ack? This is the correct term right?

/// The referenced transaction's txid
pub txid: Sha256dHash,
/// The index of the referenced output in its transaction's vout
pub index: usize
pub index: u32,
Copy link
Member

Choose a reason for hiding this comment

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

Can you call this vout instead of index?

Copy link
Member
@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Yea, nomenclature is right.

///
/// let block = genesis_block(Network::Bitcoin);
/// let tx = &block.txdata[0];
///
Copy link
Member

Choose a reason for hiding this comment

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

nit: Trailing space here

/// ```rust
/// use bitcoin::blockdata::constants::genesis_block;
/// use bitcoin::network::constants::Network;
///
Copy link
Member

Choose a reason for hiding this comment

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

nit: Trailing space here

@jeandudey jeandudey force-pushed the 2018-08-18-outpoint branch from 92a2644 to 2798609 Compare August 20, 2018 16:02
@jeandudey
Copy link
Contributor Author

Done

txid: Default::default(),
vout: u32::max_value(),
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you make this function be the Default::default implementation of the Outpoint type? You can also have the explicit method.

@apoelstra
Copy link
Member

ACK aside from default nit

Previously this structure was unused, it's now being used by the `TxIn`
structure to simplify the code a little bit and avoid confusions. Also
the rust-lightning source code has an `OutPoint` similar to this one
but with the `vout` index as an `u16` to avoid unsafe conversions.

I've added to new methods to `OutPoint`:

- `null`: Creates a new "null" `OutPoint`.
- `is_null`: Checks if the given `OutPoint` is null.

Signed-off-by: Jean Pierre Dudey <jeandudey@hotmail.com>
@jeandudey jeandudey force-pushed the 2018-08-18-outpoint branch from 2798609 to 32631e4 Compare August 20, 2018 17:46
@jeandudey
Copy link
Contributor Author

Done

@apoelstra apoelstra self-requested a review August 20, 2018 17:48
@apoelstra apoelstra added this to the 0.14 milestone Aug 20, 2018
@apoelstra apoelstra merged commit 2d96141 into rust-bitcoin:master Aug 20, 2018
@apoelstra
Copy link
Member

Normally I'd consider this too much API churn for the benefit, but I really hated the old prev_index and prev_hash names, they were nonstandard and impossible to remember.

@jeandudey jeandudey deleted the 2018-08-18-outpoint branch August 20, 2018 18:25
casey pushed a commit to casey/rust-bitcoin that referenced this pull request Nov 28, 2023
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.

3 participants
0