fix(core): anchor proofs use txType instead of version - CDB-2074 #2565
Conversation
oed
force-pushed
the
fix/tx-type
branch
2 times, most recently
from
f54d886
to
2a25852
Compare
oed
changed the title
There was a problem hiding this comment.
I suggest that we instead use txType and set it to f(bytes32) which represents any EVM function call that includes a bytes32 dag-cbor multihash
Do we want to be more specific in the type that saying it accepts any EVM function call that accepts a bytes32? Should we make clear that it must be our smart contract's anchor function? Or at least that the bytes32 must be a dag-cbor multihash?
| @@ -72,20 +70,20 @@ const getCidFromV0Transaction = (txResponse: TransactionResponse): CID => { | |||
| } | |||
|
|
|||
| const getCidFromV1Transaction = (txResponse: TransactionResponse): CID => { | |||
| const decodedArgs = iface.decodeFunctionData('anchorDagCbor', txResponse.data) | |||
There was a problem hiding this comment.
so I don't really know what this is doing, but it looks to me like using a function designed to decode the data here would be less brittle than just directly counting bytes. Any reason for changing this?
There was a problem hiding this comment.
Can change it back. I think this is safe though since it's unlikely that EVM will change the number of bytes used for the function signature. If it did it would also break the tool we are using here.
| return CID.create(1, DAG_CBOR_CODE, multihash) | ||
| } | ||
|
|
||
| /** | ||
| * Parses the transaction data to recover the CID. | ||
| * @param version version of the anchor proof. Version 1 anchor proofs are created using the official anchoring smart contract and must be parsed accordingly | ||
| * @param txType transaction type of the anchor proof. Currently support `raw` and `dagCbor(bytes32)` |
There was a problem hiding this comment.
is it dagCbor(bytes32) or f(bytes32)?
There was a problem hiding this comment.
Sorry, this should say f(bytes32).
| @@ -45,10 +44,9 @@ const BASE_CHAIN_ID = 'eip155' | |||
| const MAX_PROVIDERS_COUNT = 100 | |||
| const TRANSACTION_CACHE_SIZE = 50 | |||
| const BLOCK_CACHE_SIZE = 50 | |||
| const V0_PROOF_TYPE = 'raw' | |||
| const V1_PROOF_TYPE = 'f(bytes32)' | |||
There was a problem hiding this comment.
Can you comment what each of these strings represent/mean? f(bytes32) is pretty opaque to me (I can guess it means a function than takes 32 bytes as an input, but still not really clear what it means for that to be the "proof type" or why we chose that string)
There was a problem hiding this comment.
We should add a comment here when we've created the eip155 namespace.
Added a follow up story here: https://linear.app/3boxlabs/issue/RES-812/add-reference-to-caip-168-namespace-registered-values
I don't think we need to.
Ideally not. Would be good to keep this general as it might be used by others who use the CAIP-168 spec. The plan is to register both
We will register |
Having a
versionproperty doesn't make much sense given that we want to standardize IPLD timestamp proofs as a CAIP.I suggest that we instead use
txTypeand set it tof(bytes32)which represents any EVM function call that includes a bytes32 dag-cbor multihash. For "version 0" I suggest we useraw(and accept absence).On CAS: ceramicnetwork/ceramic-anchor-service#821