Unable to obtain sender address in postRelayedCall` or `_preRelayedCall`

:computer: Environment

I am using Open Zeppelin SDK version: 2.5.3

:memo:Details

While testing a GSN recipient transaction, the transaction gets reverted after it has been relayed by the relay hub. We did a thorough investigation into this issue and we figured out that the issue comes from our postRelayedCall and _preRelayedCall. On further investigation, we realised that the error when we try to decode the context passed to the two functions highlighted above using (address from, uint256 maxPossibleCharge) = abi.decode(context, (address, uint256));.

We want to use the postRelayedCall or _preRelayedCall to charge our users some token fees when they call certain contract methods. When we commented out the line that causes the error above and tried to use _msgSender() to obtain the address that called the contract, we got the value 0x0000000000000000000000000000000000000000 which is not what we desired. The EVM reverted the transaction.

Here is a snippet of our contract below:

    pragma solidity ^0.5.0;

    contract MyContract is GSNRecipient {
    ...

    function acceptRelayedCall(
        address relay,
        address from,
        bytes calldata encodedFunction,
        uint256 transactionFee,
        uint256 gasPrice,
        uint256 gasLimit,
        uint256 nonce,
        bytes calldata approvalData,
        uint256 maxPossibleCharge
    ) external view returns (uint256, bytes memory) {
        return acceptRelayedCall(0,"");
    }
    function _preRelayedCall(bytes memory context) internal returns (bytes32) {
        (address from, uint256 maxPossibleCharge) = abi.decode(context, (address, uint256)); //transaction reverts here
    }

    function postRelayedCall(bytes calldata context,
        bool success,
        uint actualCharge,
        bytes32 preRetVal
    ) external {
        (address from, uint256 maxPossibleCharge) = abi.decode(context, (address, uint256)); //transaction reverts here
        balances[_msgSender()] = balances[_msgSender()].sub(gsnFee); //_msgSender() gives 0x0000000...0
        balances[owner()] = balances[owner()].add(gsnFee);
        emit GSNFeeCharged(10, from);
    }
}

:1234: Code to reproduce

The code to reproduce this error can be seen below:

    const { registerRelay, deployRelayHub, fundRecipient, balance, runRelayer } = require('@openzeppelin/gsn-helpers');
    const { BN, constants, ether, expectEvent, expectRevert} = require('@openzeppelin/test-helpers');
    const gsn = require('@openzeppelin/gsn-helpers');
    const IRelayHub = artifacts.require('IRelayHub');

    const MyContract = artifacts.require('MyContract');

    contract("My Contract", function (accounts) {
    let myContract;
    const masterMinter = accounts[0];
    

    beforeEach(async function () {
        myContract = await MyContract.new();
        await myContract.initialize(...);


        await deployRelayHub(web3, {
            from: accounts[0]
        });

        await runRelayer({
            relayUrl: 'http://localhost:8090',
            workdir: process.cwd(),
            devMode: true,
            ethereumNodeURL: 'http://localhost:8545',
            gasPricePercent: 0,
            port: 8090,
            quiet: true
        });

        await registerRelay(web3, {
            relayUrl: 'http://localhost:8090',
            stake: ether('1'),
            unstakeDelay: 604800,
            funds: ether('5'),
            from: accounts[0]
        });

        await fundRecipient(web3, {
            recipient: myContract.address,
            amount: ether('2'),
            from: accounts[0]
        });

        const bal = await balance(web3, {
            recipient: myContract.address,
        });
        console.log(bal.toString());
    });

    context('when transfer is called', function () {
        beforeEach(async function () {
            await gsn.fundRecipient(web3, { recipient: myContract.address });
            this.relayHub = await IRelayHub.at('0xD216153c06E857cD7f72665E0aF1d7D82172F494');
        });

        it("should not revert transaction", async () => {
            const minter = accounts[3];
            const recipientA = accounts[2];
            
            await myContract.configureMinter(minter, 50000, { from: masterMinter });
            await myContract.mint(minter, 5000, { from: minter });
            const {tx} = await myContract.transfer(recipientA, 500, { from: minter , useGSN: true});

            await expectEvent.inTransaction(tx, IRelayHub, 'TransactionRelayed', { status: '0' });

            const senderPostBalance = await myContract.balanceOf(minter);
            expect(senderPostBalance.toString()).equal((5000-500-10).toString());
        });
    });

});
2 Likes

Hi @mayorcoded,

Welcome to the community :wave:

I recommend reading the documentation on GSN Strategies as there appear to be a few issues with MyContract:
https://docs.openzeppelin.com/contracts/2.x/gsn-strategies

I also recommend looking at GSNRecipientERC20Fee.sol as you could either use this included strategy, or base your own custom strategy on it depending on the token that you are using for payment. It might be worth starting with GSNRecipientERC20Fee first and get this working, then create your own custom strategy.

The GSN Strategies documentation covers How does GSNRecipientERC20Fee work

To write a custom strategy, simply inherit from GSNRecipient and implement the acceptRelayedCall , _preRelayedCall and _postRelayedCall functions.

In MyContract you override postRelayedCall when we should override _postRelayedCall. postRelayedCall should not be overriden (see: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/GSN/GSNRecipient.sol#L139)

Your acceptRelayedCall implementation decides whether or not to accept the relayed call: return _approveRelayedCall to accept, and return _rejectRelayedCall with an error code to reject.

In MyContract there is no logic to decide whether to accept or not accept the relayed call. acceptRelayedCall is returning acceptRelayedCall when it should return _approveRelayedCall if it wants to accept the relayed call (and return _rejectRelayedCall with an error code to reject).

As per the example of GSNRecipientERC20Fee in _preRelayedCall and _postRelayedCall MyContract should use the from rather than _msgSender()


Also if you are using OpenZeppelin SDK to create an upgradeable contract, then please note (from https://docs.openzeppelin.com/sdk/2.6/linking):

Make sure you install @openzeppelin/contracts-ethereum-package and not the vanilla @openzeppelin/contracts . The latter is set up for general usage, while @openzeppelin/contracts-ethereum-package is tailored for being used with the OpenZeppelin SDK. This means that its contracts are already set up to be upgradeable.

@abcoathup hi, @mayorcoded and I are working on the same thing. Since he posted this we’ve done everything else you just suggested. Especially looking at GSNRecipientERC20Fee.

The same thing happens, the transaction reverts immediately this line is included in _postRelayedCall

(address from, uint256 maxPossibleCharge, uint256 transactionFee, uint256 gasPrice) =
                    abi.decode(context, (address, uint256, uint256, uint256));

Getting from has just not been possible.

2 Likes

I have figured it out. @abcoathup @mayorcoded

It wasn’t immediately clear to me but one has to pass the context into _approveRelayedCall. Like in the snippet below.

2 Likes

Awesome work! @timigod

1 Like

Hi @timigod & @mayorcoded,

Glad you were able to resolve. The included strategy (and associated tests) GSNRecipientERC20Fee.sol is a great resource.

1 Like