Standardization of bytes29 libraries (#100/#119)

- All `bytes29` libraries are now enforcing strict typing for slicing methods. List of possible types: [SynapseTypes.sol](https://github.com/synapsecns/sanguine/blob/feat/bytes29-libs-standardization/packages/contracts/contracts/libs/SynapseTypes.sol)
- All `bytes29` libraries now have `isX` method, that checks whether passed payload is a properly formatted `X` (`isAttestation`, `isSystemMessage`, etc).
- Naming is now consistent across these libraries. All functions are documented.
- Existing tests are adjusted, new tests are introduced to check strict typing and proper formatting.

Co-authored-by: Trajan0x <trajan0x@users.noreply.github.com>

.codecov.yml

@@ -12,4 +12,5 @@ ignore:
 # solidity test files
  - '*/test/*.sol'
  - '*/lib/*.sol'
- - 'packages/contracts/libs/*.sol'
+ - 'packages/contracts/contracts/libs/*.sol'
+ - 'packages/contracts/test/*.sol'

core/types/encoder.go

@@ -179,9 +179,8 @@ func EncodeHeader(header Header) ([]byte, error) {
 // messageEncoder contains the binary structore of the message.
 type messageEncoder struct {
  Version uint16
- HeaderOffset uint16
- TipsOffset uint16
- BodyOffset uint16
+ HeaderLength uint16
+ TipsLength uint16
 }
 
 // EncodeMessage encodes a message.
@@ -196,16 +195,10 @@ func EncodeMessage(m Message) ([]byte, error) {
  return []byte{}, fmt.Errorf("could not encode tips: %w", err)
  }
 
- tipsOffset := headerOffset + uint16(len(encodedHeader))
- bodyOffset := tipsOffset + uint16(len(encodedTips))
-
- // payload := append(append(encodedHeader, encodedTips...), m.Body()...)
-
  newMessage := messageEncoder{
  Version: m.Version(),
- HeaderOffset: headerOffset,
- TipsOffset: tipsOffset,
- BodyOffset: bodyOffset,
+ HeaderLength: uint16(len(encodedHeader)),
+ TipsLength: uint16(len(encodedTips)),
  }
 
  buf := new(bytes.Buffer)

core/types/message.go

@@ -49,7 +49,7 @@ type messageImpl struct {
 
 const messageVersion uint16 = 1
 
-const headerOffset uint16 = 8
+const headerOffset uint16 = 6
 
 // NewMessage creates a new message from fields passed in.
 func NewMessage(header Header, tips Tips, body []byte) Message {
@@ -80,14 +80,14 @@ func DecodeMessage(message []byte) (Message, error) {
  return nil, fmt.Errorf("could not parse encoded: %w", err)
  }
 
- rawHeader := message[encoded.HeaderOffset:encoded.TipsOffset]
+ rawHeader := message[headerOffset : encoded.HeaderLength+headerOffset]
 
  header, err := DecodeHeader(rawHeader)
  if err != nil {
  return nil, fmt.Errorf("could not decode header: %w", err)
  }
 
- rawTips := message[encoded.TipsOffset:encoded.BodyOffset]
+ rawTips := message[headerOffset+encoded.HeaderLength : headerOffset+encoded.HeaderLength+encoded.TipsLength]
  unmarshalledTips, err := DecodeTips(rawTips)
  if err != nil {
  return nil, fmt.Errorf("could not decode unmarshalledTips: %w", err)
@@ -100,7 +100,7 @@ func DecodeMessage(message []byte) (Message, error) {
  return nil, fmt.Errorf("message too small, expected at least %d, got %d", dataSize, len(message))
  }
 
- rawBody := message[encoded.BodyOffset:]
+ rawBody := message[headerOffset+encoded.HeaderLength+encoded.TipsLength:]
 
  decoded := messageImpl{
  version: encoded.Version,

core/types/parity_test.go

@@ -142,9 +142,9 @@ func TestMessageEncodeParity(t *testing.T) {
  Nil(t, err)
  Equal(t, version, types.MessageVersion)
 
- headerOffset, err := messageContract.HeaderOffset(&bind.CallOpts{Context: ctx})
+ headerOffset, err := messageContract.OffsetHeader(&bind.CallOpts{Context: ctx})
  Nil(t, err)
- Equal(t, headerOffset, types.HeaderOffset)
+ Equal(t, headerOffset, big.NewInt(int64(types.HeaderOffset)))
 
  // generate some fake data
  origin := gofakeit.Uint32()
@@ -160,7 +160,7 @@ func TestMessageEncodeParity(t *testing.T) {
  proverTip := randomUint96BigInt(t)
  executorTip := randomUint96BigInt(t)
 
- formattedMessage, err := messageContract.FormatMessage(&bind.CallOpts{Context: ctx}, origin, sender, nonce, destination, recipient, optimisticSeconds, notaryTip, broadcasterTip, proverTip, executorTip, body)
+ formattedMessage, err := messageContract.FormatMessage0(&bind.CallOpts{Context: ctx}, origin, sender, nonce, destination, recipient, optimisticSeconds, notaryTip, broadcasterTip, proverTip, executorTip, body)
  Nil(t, err)
 
  decodedMessage, err := types.DecodeMessage(formattedMessage)

packages/contracts/contracts/AttestationCollector.sol

@@ -30,12 +30,12 @@ contract AttestationCollector is GlobalNotaryRegistry, OwnableUpgradeable {
  * but different root (meaning one of the attestations is fraudulent),
  * we need a system so store all such attestations.
  *
- * `attestationRoots` stores a list of attested roots for every (domain, nonce) pair
+ * `attestedRoots` stores a list of attested roots for every (domain, nonce) pair
  * `signatures` stores a signature for every submitted (domain, nonce, root) attestation.
  * We only store the first submitted signature for such attestation.
  */
  // [origin => [nonce => [roots]]]
- mapping(uint32 => mapping(uint32 => bytes32[])) internal attestationRoots;
+ mapping(uint32 => mapping(uint32 => bytes32[])) internal attestedRoots;
  // [origin => [nonce => [root => signature]]]
  mapping(uint32 => mapping(uint32 => mapping(bytes32 => bytes))) internal signatures;
 
@@ -139,8 +139,8 @@ contract AttestationCollector is GlobalNotaryRegistry, OwnableUpgradeable {
  uint32 _nonce,
  uint256 _index
  ) public view returns (bytes32) {
- require(_index < attestationRoots[_domain][_nonce].length, "!index");
- return attestationRoots[_domain][_nonce][_index];
+ require(_index < attestedRoots[_domain][_nonce].length, "!index");
+ return attestedRoots[_domain][_nonce][_index];
  }
 
  /**
@@ -149,7 +149,7 @@ contract AttestationCollector is GlobalNotaryRegistry, OwnableUpgradeable {
  * If amount > 1, fraud was committed.
  */
  function rootsAmount(uint32 _domain, uint32 _nonce) external view returns (uint256) {
- return attestationRoots[_domain][_nonce].length;
+ return attestedRoots[_domain][_nonce].length;
  }
 
  /*╔══════════════════════════════════════════════════════════════════════╗*\
@@ -207,17 +207,17 @@ contract AttestationCollector is GlobalNotaryRegistry, OwnableUpgradeable {
  }
 
  function _storeAttestation(address _notary, bytes29 _view) internal returns (bool) {
- uint32 domain = _view.attestationDomain();
- uint32 nonce = _view.attestationNonce();
- bytes32 root = _view.attestationRoot();
+ uint32 domain = _view.attestedDomain();
+ uint32 nonce = _view.attestedNonce();
+ bytes32 root = _view.attestedRoot();
  require(nonce > latestNonce[domain][_notary], "Outdated attestation");
  // Don't store Attestation, if another Notary
  // have submitted the same (domain, nonce, root) before.
  if (_signatureExists(domain, nonce, root)) return false;
  latestNonce[domain][_notary] = nonce;
  latestRoot[domain][_notary] = root;
- signatures[domain][nonce][root] = _view.attestationSignature().clone();
- attestationRoots[domain][nonce].push(root);
+ signatures[domain][nonce][root] = _view.notarySignature().clone();
+ attestedRoots[domain][nonce].push(root);
  return true;
  }
 }

packages/contracts/contracts/Destination.sol

@@ -12,7 +12,7 @@ import { Message } from "./libs/Message.sol";
 import { Header } from "./libs/Header.sol";
 import { Tips } from "./libs/Tips.sol";
 import { TypeCasts } from "./libs/TypeCasts.sol";
-import { SystemMessage } from "./system/SystemMessage.sol";
+import { SystemMessage } from "./libs/SystemMessage.sol";
 import { SystemContract } from "./system/SystemContract.sol";
 import { IMessageRecipient } from "./interfaces/IMessageRecipient.sol";
 // ============ External Imports ============
@@ -146,21 +146,16 @@ contract Destination is Version0, SystemContract, GlobalNotaryRegistry, GuardReg
  */
  function submitAttestation(bytes memory _attestation) external {
  (, bytes29 _view) = _checkNotaryAuth(_attestation);
- uint32 remoteDomain = _view.attestationDomain();
+ uint32 remoteDomain = _view.attestedDomain();
  require(remoteDomain != localDomain, "Attestation refers to local chain");
- uint32 nonce = _view.attestationNonce();
+ uint32 nonce = _view.attestedNonce();
  MirrorLib.Mirror storage mirror = allMirrors[activeMirrors[remoteDomain]];
  require(nonce > mirror.nonce, "Attestation older than current state");
- bytes32 newRoot = _view.attestationRoot();
+ bytes32 newRoot = _view.attestedRoot();
  mirror.setConfirmAt(newRoot, block.timestamp);
  // update nonce
  mirror.setNonce(nonce);
- emit AttestationAccepted(
- remoteDomain,
- nonce,
- newRoot,
- _view.attestationSignature().clone()
- );
+ emit AttestationAccepted(remoteDomain, nonce, newRoot, _view.notarySignature().clone());
  }
 
  /**
@@ -192,35 +187,35 @@ contract Destination is Version0, SystemContract, GlobalNotaryRegistry, GuardReg
  * @param _message Formatted message
  */
  function execute(bytes memory _message) public {
- bytes29 _m = _message.messageView();
- bytes29 _header = _m.header();
- uint32 _remoteDomain = _header.origin();
- MirrorLib.Mirror storage mirror = allMirrors[activeMirrors[_remoteDomain]];
+ bytes29 messageView = _message.castToMessage();
+ bytes29 header = messageView.header();
+ uint32 remoteDomain = header.origin();
+ MirrorLib.Mirror storage mirror = allMirrors[activeMirrors[remoteDomain]];
  // ensure message was meant for this domain
- require(_header.destination() == localDomain, "!destination");
+ require(header.destination() == localDomain, "!destination");
  // ensure message has been proven
- bytes32 _messageHash = _m.keccak();
- bytes32 _root = mirror.messageStatus[_messageHash];
- require(MirrorLib.isPotentialRoot(_root), "!exists || executed");
+ bytes32 messageHash = messageView.keccak();
+ bytes32 root = mirror.messageStatus[messageHash];
+ require(MirrorLib.isPotentialRoot(root), "!exists || executed");
  require(
- acceptableRoot(_remoteDomain, _header.optimisticSeconds(), _root),
+ acceptableRoot(remoteDomain, header.optimisticSeconds(), root),
  "!optimisticSeconds"
  );
  // check re-entrancy guard
  require(entered == 1, "!reentrant");
  entered = 0;
- _storeTips(_m.tips());
+ _storeTips(messageView.tips());
  // update message status as executed
- mirror.setMessageStatus(_messageHash, MirrorLib.MESSAGE_STATUS_EXECUTED);
- address recipient = _checkForSystemMessage(_header.recipient());
+ mirror.setMessageStatus(messageHash, MirrorLib.MESSAGE_STATUS_EXECUTED);
+ address recipient = _checkForSystemMessage(header.recipient());
  IMessageRecipient(recipient).handle(
- _remoteDomain,
- _header.nonce(),
- _header.sender(),
- mirror.confirmAt[_root],
- _m.body().clone()
+ remoteDomain,
+ header.nonce(),
+ header.sender(),
+ mirror.confirmAt[root],
+ messageView.body().clone()
  );
- emit Executed(_remoteDomain, _messageHash);
+ emit Executed(remoteDomain, messageHash);
  // reset re-entrancy guard
  entered = 1;
  }

packages/contracts/contracts/Origin.sol

@@ -10,7 +10,7 @@ import { MerkleLib } from "./libs/Merkle.sol";
 import { Header } from "./libs/Header.sol";
 import { Message } from "./libs/Message.sol";
 import { Tips } from "./libs/Tips.sol";
-import { SystemMessage } from "./system/SystemMessage.sol";
+import { SystemMessage } from "./libs/SystemMessage.sol";
 import { SystemContract } from "./system/SystemContract.sol";
 import { MerkleTreeManager } from "./Merkle.sol";
 import { INotaryManager } from "./interfaces/INotaryManager.sol";
@@ -208,7 +208,7 @@ contract Origin is
  bytes memory _messageBody
  ) external payable notFailed {
  require(_messageBody.length <= MAX_MESSAGE_BODY_BYTES, "msg too long");
- require(_tips.tipsView().totalTips() == msg.value, "!tips");
+ require(_tips.castToTips().totalTips() == msg.value, "!tips");
  // get the next nonce for the destination domain, then increment it
  nonce = nonce + 1;
  bytes32 _sender = _checkForSystemMessage(_recipientAddress);
@@ -279,8 +279,8 @@ contract Origin is
  function improperAttestation(bytes memory _attestation) public notFailed returns (bool) {
  // This will revert if signature is not valid
  (address _notary, bytes29 _view) = _checkNotaryAuth(_attestation);
- uint32 _nonce = _view.attestationNonce();
- bytes32 _root = _view.attestationRoot();
+ uint32 _nonce = _view.attestedNonce();
+ bytes32 _root = _view.attestedRoot();
  // Check if nonce is valid, if not => attestation is fraud
  if (_nonce < historicalRoots.length) {
  if (_root == historicalRoots[_nonce]) {

packages/contracts/contracts/libs/Attestation.sol

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.13;
 
+import { SynapseTypes } from "./SynapseTypes.sol";
 import { TypedMemView } from "./TypedMemView.sol";
 
 library Attestation {
@@ -24,8 +25,28 @@ library Attestation {
  uint256 internal constant ATTESTATION_DATA_LENGTH = 40;
  uint256 internal constant OFFSET_SIGNATURE = ATTESTATION_DATA_LENGTH;
 
+ /*╔══════════════════════════════════════════════════════════════════════╗*\
+ ▏*║ MODIFIERS ║*▕
+ \*╚══════════════════════════════════════════════════════════════════════╝*/
+
+ modifier onlyAttestation(bytes29 _view) {
+ _view.assertType(SynapseTypes.ATTESTATION);
+ _;
+ }
+
+ /*╔══════════════════════════════════════════════════════════════════════╗*\
+ ▏*║ FORMATTERS ║*▕
+ \*╚══════════════════════════════════════════════════════════════════════╝*/
+
  /**
- * @notice Returns formatted Attestation with provided fields
+ * @notice Returns a properly typed bytes29 pointer for an attestation payload.
+ */
+ function castToAttestation(bytes memory _payload) internal pure returns (bytes29) {
+ return _payload.ref(SynapseTypes.ATTESTATION);
+ }
+
+ /**
+ * @notice Returns a formatted Attestation payload with provided fields
  * @param _data Attestation Data (see above)
  * @param _signature Notary's signature on `_data`
  * @return Formatted attestation
@@ -39,11 +60,11 @@ library Attestation {
  }
 
  /**
- * @notice Returns formatted Attestation Data with provided fields
+ * @notice Returns a formatted AttestationData payload with provided fields
  * @param _domain Domain of Origin's chain
  * @param _root New merkle root
  * @param _nonce Nonce of the merkle root
- * @return Formatted data
+ * @return Formatted attestation data
  **/
  function formatAttestationData(
  uint32 _domain,
@@ -54,45 +75,55 @@ library Attestation {
  }
 
  /**
- * @notice Checks that message is an Attestation, by checking its length
+ * @notice Checks that a payload is a formatted Attestation payload.
  */
  function isAttestation(bytes29 _view) internal pure returns (bool) {
- // Should have non-zero length for signature. Signature validity is not checked.
+ // Should have at least 1 bytes for the signature.
+ // Signature length or validity is not checked, ECDSA.sol takes care of it.
  return _view.len() > ATTESTATION_DATA_LENGTH;
  }
 
+ /*╔══════════════════════════════════════════════════════════════════════╗*\
+ ▏*║ ATTESTATION SLICING ║*▕
+ \*╚══════════════════════════════════════════════════════════════════════╝*/
+
  /**
  * @notice Returns domain of chain where the Origin contract is deployed
  */
- function attestationDomain(bytes29 _view) internal pure returns (uint32) {
+ function attestedDomain(bytes29 _view) internal pure onlyAttestation(_view) returns (uint32) {
  return uint32(_view.indexUint(OFFSET_ORIGIN_DOMAIN, 4));
  }
 
  /**
  * @notice Returns nonce of Origin contract at the time, when `root` was the Merkle root.
  */
- function attestationNonce(bytes29 _view) internal pure returns (uint32) {
+ function attestedNonce(bytes29 _view) internal pure onlyAttestation(_view) returns (uint32) {
  return uint32(_view.indexUint(OFFSET_NONCE, 4));
  }
 
  /**
  * @notice Returns a historical Merkle root from the Origin contract
  */
- function attestationRoot(bytes29 _view) internal pure returns (bytes32) {
+ function attestedRoot(bytes29 _view) internal pure onlyAttestation(_view) returns (bytes32) {
  return _view.index(OFFSET_ROOT, 32);
  }
 
  /**
  * @notice Returns Attestation's Data, that is going to be signed by the Notary
  */
- function attestationData(bytes29 _view) internal pure returns (bytes29) {
- return _view.slice(OFFSET_ORIGIN_DOMAIN, ATTESTATION_DATA_LENGTH, 0);
+ function attestationData(bytes29 _view) internal pure onlyAttestation(_view) returns (bytes29) {
+ return
+ _view.slice(
+ OFFSET_ORIGIN_DOMAIN,
+ ATTESTATION_DATA_LENGTH,
+ SynapseTypes.ATTESTATION_DATA
+ );
  }
 
  /**
  * @notice Returns Notary's signature on AttestationData
  */
- function attestationSignature(bytes29 _view) internal pure returns (bytes29) {
- return _view.slice(OFFSET_SIGNATURE, _view.len() - ATTESTATION_DATA_LENGTH, 0);
+ function notarySignature(bytes29 _view) internal pure onlyAttestation(_view) returns (bytes29) {
+ return _view.sliceFrom(OFFSET_SIGNATURE, SynapseTypes.SIGNATURE);
  }
 }