-
Notifications
You must be signed in to change notification settings - Fork 49
Implement GetRawMempoolSequence return type for getrawmempool RPC
#478
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
Conversation
types/src/model/blockchain.rs
Outdated
| /// List of transaction ids in the mempool. | ||
| pub txids: Vec<Txid>, | ||
| /// The mempool sequence value. | ||
| pub mempool_sequence: bitcoin::Sequence, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pub mempool_sequence: bitcoin::Sequence, | |
| pub mempool_sequence: u64, |
bitcoin::Sequence is a transaction input sequence.
types/src/v21/blockchain/mod.rs
Outdated
| /// List of transaction ids in the mempool. | ||
| pub txids: Vec<String>, | ||
| /// The mempool sequence value. | ||
| pub mempool_sequence: u32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pub mempool_sequence: u32, | |
| pub mempool_sequence: u64, |
I think a mempool sequence is u64 in Core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed it is u64 and made the change.
3e8a0b4 to
4928178
Compare
jamillambert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 4928178
types/src/v21/blockchain/into.rs
Outdated
| // Parse txid strings into `bitcoin::Txid`. | ||
| let txids = | ||
| self.txids.iter().map(|t: &String| t.parse::<Txid>()).collect::<Result<Vec<_>, _>>()?; | ||
|
|
||
| Ok(model::GetRawMempoolSequence { txids, mempool_sequence: self.mempool_sequence }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be more simply written as
let txids = self.txids.iter().map(|t| t.parse::<Txid>()).collect::<Result<Vec<_>, _>>()?;
Ok(model::GetRawMempoolSequence { txids, mempool_sequence: self.mempool_sequence })| } | ||
| pub fn get_raw_mempool_verbose(&self) -> Result<GetRawMempoolVerbose> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: A line of whitespace between the functions here please mate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything else looks good. Thanks man. Reviewed: 4928178
When getrawmempool is called with verbose set to false and mempool_sequence set to true, a distinct type is returned. This commit implements that return type with test, from version v21 through v30.
4928178 to
7706c36
Compare
jamillambert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 7706c36
tcharding
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 7706c36
When
getrawmempoolRPC is called with optional verbose argument set tofalseand subsequentmempool_sequenceset totrue, a distinct type is returned from v21 onward to v30. This PR implements this return type with corresponding tests.Partially fixes issue #474.