From f1c97c2fc5de106d8f2150e85f2c9824b3ed0d4c Mon Sep 17 00:00:00 2001
From: Chad Ostrowski <221614+chadoh@users.noreply.github.com>
Date: Wed, 20 May 2020 17:02:30 -0400
Subject: [PATCH] feat!: mutually exclude transfer & transfer_from

A recommendation for an improvement to the NEP-4 spec at
https://github.com/nearprotocol/NEPs/pull/4

The change:

    -// * The caller of the function (`predecessor`) should have access to the token.
    +// * The caller of the function (`predecessor`) should own the token.
     export function transfer(new_owner_id: string, token_id: TokenId): void

BREAKING CHANGE: this intentionally deviates from the current spec
---
 .../nep4/__tests__/main.unit.spec.ts          | 21 ++++++++++++++-----
 contracts/assemblyscript/nep4/main.ts         | 11 ++++++----
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/contracts/assemblyscript/nep4/__tests__/main.unit.spec.ts b/contracts/assemblyscript/nep4/__tests__/main.unit.spec.ts
index 86bf714..0d4e767 100644
--- a/contracts/assemblyscript/nep4/__tests__/main.unit.spec.ts
+++ b/contracts/assemblyscript/nep4/__tests__/main.unit.spec.ts
@@ -12,6 +12,7 @@ import { VM, Context } from 'near-sdk-as'
 
 const alice = 'alice'
 const bob = 'bob'
+const carol = 'carol'
 
 describe('grant_access', () => {
   it('grants access to the given account_id for all the tokens that account has', () => {
@@ -58,15 +59,16 @@ describe('revoke_access', () => {
 describe('transfer_from', () => {
   it('transfers the given `token_id` from the given `owner_id` to  `new_owner_id`', () => {
     const aliceToken = nonSpec.mint_to(alice)
-    Context.setPredecessor_account_id(alice)
 
-    expect(get_token_owner(aliceToken)).toBe(alice)
-    expect(get_token_owner(aliceToken)).not.toBe(bob)
+    Context.setPredecessor_account_id(alice)
+    grant_access(bob)
 
-    transfer_from(alice, bob, aliceToken)
+    Context.setPredecessor_account_id(bob)
+    transfer_from(alice, carol, aliceToken)
 
-    expect(get_token_owner(aliceToken)).toBe(bob)
+    expect(get_token_owner(aliceToken)).toBe(carol)
     expect(get_token_owner(aliceToken)).not.toBe(alice)
+    expect(get_token_owner(aliceToken)).not.toBe(bob)
   })
 
   it('requires the caller of the function to have access to the token.', () => {
@@ -77,6 +79,15 @@ describe('transfer_from', () => {
       transfer_from(alice, bob, aliceToken);
     }).toThrow(nonSpec.ERROR_CALLER_ID_DOES_NOT_MATCH_EXPECTATION)
   })
+
+  it('requires the caller of the function to not have ownership of the token', () => {
+    expect(() => {
+      const aliceToken = nonSpec.mint_to(alice)
+      Context.setPredecessor_account_id(alice)
+
+      transfer_from(alice, bob, aliceToken);
+    }).toThrow(nonSpec.ERROR_CALLER_ID_DOES_NOT_MATCH_EXPECTATION)
+  })
 })
 
 describe('transfer', () => {
diff --git a/contracts/assemblyscript/nep4/main.ts b/contracts/assemblyscript/nep4/main.ts
index 48cc609..24ebb5c 100644
--- a/contracts/assemblyscript/nep4/main.ts
+++ b/contracts/assemblyscript/nep4/main.ts
@@ -75,15 +75,18 @@ export function revoke_access(escrow_account_id: string): void {
 // * The caller of the function (`predecessor`) should have access to the token.
 export function transfer_from(owner_id: string, new_owner_id: string, token_id: TokenId): void {
   const predecessor = context.predecessor
-  assert(owner_id == predecessor, ERROR_CALLER_ID_DOES_NOT_MATCH_EXPECTATION)
 
-  // delegate to transfer function since this seems redundant
-  transfer(new_owner_id, token_id)
+  // fetch token escrow & verify access
+  const escrowAccounts = escrowAccess.getSome(owner_id)
+  assert(escrowAccounts.includes(predecessor), ERROR_CALLER_ID_DOES_NOT_MATCH_EXPECTATION)
+
+  // assign new owner to token
+  tokenToOwner.set(token_id, new_owner_id)
 }
 
 // Transfer the given `token_id` to the given `new_owner_id`.  Account `new_owner_id` becomes the new owner.
 // Requirements:
-// * The caller of the function (`predecessor`) should have access to the token.
+// * The caller of the function (`predecessor`) should own the token.
 export function transfer(new_owner_id: string, token_id: TokenId): void {
   const predecessor = context.predecessor