Anchoring elements are not stored in the anchor contract

[Therecanbeonlyone1969]

The current SimpleSidetreeAnchor.sol does not store the anchoring elements in the smart contract such that they could be verified by a 3rd party. Suggest to amend the contract the following way:

`pragma solidity 0.5.0;

contract SimpleSidetreeAnchor {
struct Anchor {
address operator;
bytes32 anchorhash;
string anchorURI;
}

mapping(uint256 => Anchor) anchor;

uint256 public transactionNumber = 0;
event Anchor(address indexed operator. bytes32 anchorFileHash, string anchoruri, uint256 transactionNumber);
function anchoringHash(bytes32 _anchorHash, string _anchorURI) public { 
    transactionNumber = transactionNumber + 1;
    anchor[transactionNumber].operator = msg.sender;
    anchor[transactionNumber].anchorhash = _anchorHash;
    anchor[transactionNumber].anchorURI = _anchorURI;
    emit Anchor(msg.sender,_anchorHash, _anchorURI, transactionNumber);
}

function getAnchorHash (uint256 transactionnumber) public view returns (address, bytes32, string)

   if (transctionnumber >0) {
      return (anchor[transactionnumber].operator, anchor[transactionnumber].anchorhash, anchor[transactionnumber].anchorURI);  

} else {
return (anchor[transactionNumber].operator, anchor[transactionNumber].anchorhash, anchor[transactionNumber].anchorURI);
}
}`

[OR13]

@Therecanbeonlyone1969 thanks for raising this.

Can we clarify what we mean by 3rd party, I assume given the contract changes, the desire is to be able to write smart contracts that interact with the anchoring more directly?

The current model is definitely designed with the intention of not inviting contracts to do anything, and the reason for this is to keep the ethereum anchoring as close to the bitcoin anchoring as possible.

Another thing to note, is that the side of anchorFileURI is now larger than bytes32... we will need to adjust for that as well...

cc @gjgd

[Therecanbeonlyone1969]

@OR13 3rd party refers to anyone (non-contract) ... we could add a check to ensure that msg.sender is not a contract -- there is an open-zeppelin contract for that ... we could just extract the function. That is a good point.

import "./openzeppelin-contracts-master/contracts/utils/Address.sol"; to be added into the contract

and in the anchoringHash function we can add at the beginning before we increase the transaction number the following check
require(!msg.sender.isContract(),"Sender cannot be a contract");

That would solve the problem that a contract can call the function ... except in the case that the contract is called in another contract constructor.

Thoughts?

[Therecanbeonlyone1969]

oh and the anchor file URI is strong so we can make that longer @OR13

[gjgd]

We went with the event based approach instead of storing in the anchor contract because storing values is significantly more expensive than to emit an event in terms of gas.

We use web3's getPastEvents to get the data emitted by events.

Also, can you clarify your distincting between anchorHash and anchorURI ?

[Therecanbeonlyone1969]

I understand the approach to use log storage to avoid paying for gas. With 1.X that approach will no longer work since the logs will be flushed after a certain length of time in order to reduce the data that has to be stored by a client. Hence, to future proof the design, I would highly recommend to pay for the gas -- not that much anyway.

The URI is the IPFS hash such that I can get to the leafs and recompute the roothash.

[gjgd]

The URI is the IPFS hash such that I can get to the leafs and recompute the roothash.

Isn't that the anchorHash? Why do you have two hashes?

[Therecanbeonlyone1969]

I was under the impression that we have both the IPFS hash and the Merkle Trie Roothash at our disposal ... this would provide two anchor points.

[gjgd]

We just write the anchorFileHash to the smart contract, which itself contain the reference to the merkle tree hash and the batch file hash (containing operation data). This 2 file structure allows us to have just one anchor hash.

See this transaction for example: https://element-did.com/server/transactions/0xabfdcf79494c3a36072ecb641cc049e27cf335be4218771852bb1c2564e6456b

• Element Transaction 1566's anchor file hash is: QmYCmcpPB1KwXxbpwtRfu7SksUCmhtz4eWCuitZxTHFcYc
• The anchor file above contains both:
  • the Merkle root (I assume this is the one you're referring to?)
  • the IPFS hash of the batch file: QmRJUHGmGorcLdFqeczaie39EJpuQByjhS7DaWAu9D5S4Y
• The batch file contains all the operation data, that you can verify with the merkle root

[OR13]

@Therecanbeonlyone1969 The Merkle Trie stuff was removed from the latest spec: https://identity.foundation/sidetree/spec/

The general design considerations for the previous contract were the following:

1. Make it as cheap as possible to anchor IPFS data, to support users who don't want to batch
2. Anything that helps reading events in order from a performance side is a MUST
3. Assume that the attacker has lots of funds, rely on the mechanics of ledger security, not offchain game theoretics structures (this is in conflict with 1, and Microsoft's approach with ION).
4. keep the ethereum side of "element" as small as possibly
5. avoid contract complexity / attack surface.

I think in the next version of the contract, we want to consider:

1. should we offer support for the value staking functionality that ION has in the same or a separate contract.
2. can we make reading of events faster (how?)
3. do we have enough space to store the new anchorFileStrings, but not any more than is needed.
4. can we make the contract calls as cheap as possible.

[Therecanbeonlyone1969]

@OR13 ... ok. need to read the latest spec.

totally understand the design criteria for v1 of the contract.

I think we need to account for in the new design:

• future-proofing the design -- such as ensuring that we are not using storage that will be used differently or no longer going forward

• the value staking (see issue Current element contract has no sybil attack protection #197 I opened yesterday with the proposal)

• reads of the contract if we have just a get that is fast, does not cost gas and depends on the capacity of the client

• we just need to use a string for the anchor hash instead of bytes32 (more costly but not outrageous and would be subsumed into the service fee of the operator)

• if they are Gets no issue, writes will cost but i think we need the extra structure to put a rate limiter in as suggested in the proposal in issue Current element contract has no sybil attack protection #197 -- the approach discussed there makes attacks even with large funds available very costly very quickly

• putting both registry and staking functions into one contract, that reduces the attack surface, but makes it not upgradeable -- trade-offs we can discuss in more details

Lastly, I am not so sure that we should not make becoming an operator for others more expensive -- we could have a simpler version of the contract that non-operator nodes who want to run their own thing can use but that is then at their own risk.My 2 wei.

[OR13]

I left a comment here: #197 (comment)

If we can, I'd prefer to adapt something general purpose and proven to this problem, rather than innovate here.