Merge pull request #2 from tradingstrategy-ai/emit-message-payload

- Log the full signing message in the event logs when the terms of service is updated
- Fix Solidity compiler version (pin down was not high enough)
- Document test suite discovery broken with `ape test`

README.md

@@ -74,7 +74,9 @@ ape compile
 
 ```shell
 poetry shell
-ape test
+# TODO: there is a bug in pytest/ape test that it does not find the tests
+# unless you explicitly pass it the tests folder
+ape test tests
 ```
 
 ## Deploying

src/TermsOfService.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-3.0
 
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.23;
 
 import "@openzeppelin/access/Ownable.sol";
 
@@ -9,8 +9,6 @@ import "@openzeppelin/access/Ownable.sol";
 import "@openzeppelin/utils/cryptography/SignatureChecker.sol";
 
 
-
-
 /**
  * Minimal MVP interface
  */
@@ -26,6 +24,7 @@ interface ITermsOfService {
  */
 contract TermsOfService is Ownable, ITermsOfService {
 
+    // OpenZeppelin lib supporting smart contract based signing as opposite to EOA signing
     using SignatureChecker for address;
 
     // Terms of service acceptances
@@ -39,19 +38,26 @@ contract TermsOfService is Ownable, ITermsOfService {
     //
     // Published terms of services
     //
+    //
+    // Terms of service versions, starting from 1 and
+    // increased with one for the each iteration.
+    //
     mapping(uint16 version => bytes32 acceptanceMessageHash) public versions;
 
+    //
+    // What is the hash of currently active terms of service version
+    //
     bytes32 public latestAcceptanceMessageHash;
 
     //
-    // Terms of service versions, starting from 1 and
-    // increased with one for the each iteration.
+    // What is the version numbe of current terms of service version
     //
     uint16 public latestTermsOfServiceVersion;
 
     // Add a new terms of service version
-    event UpdateTermsOfService(uint16 version, bytes32 acceptanceMessageHash);
+    event UpdateTermsOfService(uint16 version, bytes32 acceptanceMessageHash, string acceptanceMessage);
 
+    // User accepted a specific terms of service version
     event Signed(address signer, uint16 version, bytes32 hash, bytes metadata);
 
     constructor() Ownable() {
@@ -71,13 +77,20 @@ contract TermsOfService is Ownable, ITermsOfService {
         return hasAcceptedHash(account, hash);
     }
 
-    function updateTermsOfService(uint16 version, bytes32 acceptanceMessageHash) public onlyOwner {
+    /**
+     * Update to the new terms of service version.
+     *
+     * We do not check whether acceptance message hash matches the message,
+     * as the message string here is just for the event log keeping.
+     *
+     */
+    function updateTermsOfService(uint16 version, bytes32 acceptanceMessageHash, string calldata acceptanceMessage) public onlyOwner {
         require(version == latestTermsOfServiceVersion + 1, "Versions must be updated incrementally");
         require(acceptanceMessageHash != latestAcceptanceMessageHash, "Setting the same terms of service twice");
         latestAcceptanceMessageHash = acceptanceMessageHash;
         latestTermsOfServiceVersion = version;
         versions[version] = acceptanceMessageHash;
-        emit UpdateTermsOfService(version, acceptanceMessageHash);
+        emit UpdateTermsOfService(version, acceptanceMessageHash, acceptanceMessage);
     }
 
     /**

tests/test_sign_terms_of_service.py

@@ -33,7 +33,7 @@ def test_not_signed(
 ):
     new_version = 1
     new_hash = random.randbytes(32)
-    tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+    tos.updateTermsOfService(new_version, new_hash, "", sender=deployer)
 
     assert not tos.hasAcceptedHash(random_user, new_hash)
     assert not tos.hasAcceptedVersion(random_user, 1)
@@ -56,7 +56,7 @@ def test_signed(
     )
     new_hash = get_signing_hash(signing_content)
 
-    tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+    tos.updateTermsOfService(new_version, new_hash, signing_content, sender=deployer)
 
     assert not tos.hasAcceptedHash(random_user, new_hash)
     assert not tos.hasAcceptedVersion(random_user, 1)
@@ -94,7 +94,7 @@ def test_sign_twice(
     )
     new_hash = get_signing_hash(signing_content)
 
-    tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+    tos.updateTermsOfService(new_version, new_hash, signing_content, sender=deployer)
 
     signature = random_user.sign_message(signing_content).encode_rsv()
 
@@ -120,7 +120,7 @@ def test_sign_no_version(
     )
     new_hash = get_signing_hash(signing_content)
 
-    tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+    tos.updateTermsOfService(new_version, new_hash, signing_content, sender=deployer)
 
     signature = random_user.sign_message(signing_content).encode_rsv()
 
@@ -144,7 +144,7 @@ def test_sign_wrong_signature(
     )
     new_hash = get_signing_hash(signing_content)
 
-    tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+    tos.updateTermsOfService(new_version, new_hash, signing_content, sender=deployer)
 
     with reverts("Signature is not valid"):
         tos.signTermsOfServiceOwn(new_hash, random.randbytes(32), b"", sender=random_user)

tests/test_update_terms_of_service.py

@@ -32,7 +32,7 @@ def test_start_zero_version(tos: ContractInstance):
 def test_first_terms_of_service(tos: ContractInstance, deployer: TestAccount):
     new_version = 1
     new_hash = random.randbytes(32)
-    tx = tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+    tx = tos.updateTermsOfService(new_version, new_hash, "", sender=deployer)
     assert tos.latestTermsOfServiceVersion() == 1
     assert tos.latestAcceptanceMessageHash() == new_hash
 
@@ -48,13 +48,13 @@ def test_first_terms_of_service(tos: ContractInstance, deployer: TestAccount):
 def test_second_terms_of_service(tos: ContractInstance, deployer: TestAccount):
     new_version = 1
     new_hash = random.randbytes(32)
-    tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+    tos.updateTermsOfService(new_version, new_hash, "", sender=deployer)
     assert tos.latestTermsOfServiceVersion() == 1
     assert tos.latestAcceptanceMessageHash() == new_hash
 
     new_version = 2
     new_hash = random.randbytes(32)
-    tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+    tos.updateTermsOfService(new_version, new_hash, "", sender=deployer)
     assert tos.latestTermsOfServiceVersion() == 2
     assert tos.latestAcceptanceMessageHash() == new_hash
 
@@ -64,7 +64,7 @@ def test_wrong_owner(tos: ContractInstance, random_user: TestAccount):
     new_hash = random.randbytes(32)
 
     with pytest.raises(ContractLogicError):
-        tos.updateTermsOfService(new_version, new_hash, sender=random_user)
+        tos.updateTermsOfService(new_version, new_hash, "", sender=random_user)
 
     assert tos.latestTermsOfServiceVersion() == 0
 
@@ -74,19 +74,19 @@ def test_wrong_version(tos: ContractInstance, deployer: TestAccount):
     new_hash = random.randbytes(32)
 
     with pytest.raises(ContractLogicError):
-        tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+        tos.updateTermsOfService(new_version, new_hash, "", sender=deployer)
 
     assert tos.latestTermsOfServiceVersion() == 0
 
 
 def test_hash_reuse(tos: ContractInstance, deployer: TestAccount):
     new_version = 1
     new_hash = random.randbytes(32)
-    tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+    tos.updateTermsOfService(new_version, new_hash, "", sender=deployer)
 
     new_version = 2
     with pytest.raises(ContractLogicError):
-        tos.updateTermsOfService(new_version, new_hash, sender=deployer)
+        tos.updateTermsOfService(new_version, new_hash, "", sender=deployer)
 
 
 def test_transfer_ownership(tos: ContractInstance, deployer: TestAccount, random_user: TestAccount):