-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Storage chains: serve transactions over IPFS/bitswap #7963
Conversation
@tomaka please review or reassign |
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.
I'm not familiar with the bitswap protocol, so I can't tell if the messages being exchanged are correct, but the code looks good to me!
I've tested this against js-ipfs. |
Co-authored-by: Pierre Krieger <[email protected]>
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.
LGTM.
I wonder a bit if big tx should be split in multiple block.
@@ -95,6 +95,17 @@ pub trait BlockBackend<Block: BlockT> { | |||
|
|||
/// Get block hash by number. | |||
fn block_hash(&self, number: NumberFor<Block>) -> sp_blockchain::Result<Option<Block::Hash>>; | |||
|
|||
/// Get single extrinsic by hash. |
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.
Should we name the method transaction.
Also wondering if we should comment the fact that in some case do not have result depending on node configuration (in previous pr column 'TRANSACTION' was only optionally use).
client/api/src/in_mem.rs
Outdated
&self, | ||
_hash: &Block::Hash, | ||
) -> sp_blockchain::Result<Option<<Block as BlockT>::Extrinsic>> { | ||
unimplemented!() |
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.
will it be implemented later, if not maybe add a parameter message saying that it is unsupported.
bot merge |
Trying merge. |
See #7962
Initially I've tried implementing this as request-response protocol, but found out it is not suitable due to paritytech/polkadot-sdk#542.
polkadot companion: paritytech/polkadot#2315