Skip to content
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

An alternative approach to AnchorFromBlockPosition. #1266

Closed
evanlinjin opened this issue Jan 11, 2024 · 1 comment · Fixed by #1594
Closed

An alternative approach to AnchorFromBlockPosition. #1266

evanlinjin opened this issue Jan 11, 2024 · 1 comment · Fixed by #1594
Assignees
Labels
discussion There's still a discussion ongoing good first issue Good for newcomers module-blockchain

Comments

@evanlinjin
Copy link
Member

evanlinjin commented Jan 11, 2024

This was done previously in #1041 but I missed it. Why can't we just have BlockPosition: Into<A> bound instead of adding our own conversion trait?

Originally posted by @LLFourn in #1172 (comment)

@evanlinjin's response:

It's because there are 3 inputs and doing A: From<(&Block, BlockId, usize)> is confusing and we won't know what the usize is for.

An alternative would be to have:

/// Parameters for constructing an [`Anchor`].
pub struct AnchorParams<'b> {
    pub block: &'b Block,
    pub block_id: BlockId,
    pub tx_pos: usize,
}

fn some_method<A: Anchor + From<AnchorParams>>() { todo!() }

What do you think? (AnchorParams will be nicely documented of course!)

@evanlinjin evanlinjin added the discussion There's still a discussion ongoing label Jan 11, 2024
@evanlinjin evanlinjin added this to the 1.0.0 milestone Jan 16, 2024
@LLFourn
Copy link
Contributor

LLFourn commented Jan 22, 2024

I think the "alternative" is what I was suggesting. AnchorParams looks like a worse named version of BlockPosition which is what I suggested originally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion There's still a discussion ongoing good first issue Good for newcomers module-blockchain
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants