Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: forbid ds owner from subscriptions and consumer management calls #125

Open
wants to merge 1 commit into
base: development
Choose a base branch
from
Open

fix: forbid ds owner from subscriptions and consumer management calls #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions contracts/subscription/GenericSingleDatasetSubscriptionManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
error ARRAY_LENGTH_MISMATCH();
error NOTHING_TO_PAY();
error SUBSCRIPTION_FEE_EXCEEDS_LIMIT();
error DATASET_OWNER_FORBIDDEN(address account);

struct SubscriptionDetails {
uint256 validSince;
Expand Down Expand Up @@ -78,6 +79,12 @@ abstract contract GenericSingleDatasetSubscriptionManager is
_;
}

modifier forbidDatasetOwner() {
address msgSender = _msgSender();
if (dataset.ownerOf(datasetId) == msgSender) revert DATASET_OWNER_FORBIDDEN(msgSender);
_;
}

/**
* @notice Initialization function
* @param dataset_ The address of the DatasetNFT contract
Expand Down Expand Up @@ -124,6 +131,9 @@ abstract contract GenericSingleDatasetSubscriptionManager is
*/
function isSubscriptionPaidFor(uint256 ds, address consumer) external view returns (bool) {
_requireCorrectDataset(ds);

if (dataset.ownerOf(datasetId) == consumer) return true;

EnumerableSet.UintSet storage subscrs = _consumerSubscriptions[consumer];
uint256 totalSubscribers = subscrs.length();
for (uint256 i; i < totalSubscribers; ) {
Expand Down Expand Up @@ -196,7 +206,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
uint256 durationInDays,
uint256 consumers,
uint256 maxFee
) external payable returns (uint256 sid) {
) external payable forbidDatasetOwner returns (uint256 sid) {
return _subscribe(ds, durationInDays, consumers, maxFee);
}

Expand All @@ -221,7 +231,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
uint256 durationInDays,
address[] calldata consumers,
uint256 maxFee
) external payable returns (uint256 sid) {
) external payable forbidDatasetOwner returns (uint256 sid) {
sid = _subscribe(ds, durationInDays, consumers.length, maxFee);
_addConsumers(sid, consumers);
}
Expand Down Expand Up @@ -257,7 +267,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
uint256 extraDurationInDays,
uint256 extraConsumers,
uint256 maxExtraFee
) external payable {
) external payable forbidDatasetOwner {
_extendSubscription(subscription, extraDurationInDays, extraConsumers, maxExtraFee);
}

Expand Down Expand Up @@ -293,7 +303,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
uint256 extraDurationInDays,
address[] calldata extraConsumers,
uint256 maxExtraFee
) external payable {
) external payable forbidDatasetOwner {
_extendSubscription(subscription, extraDurationInDays, extraConsumers.length, maxExtraFee);
if (extraConsumers.length > 0) {
_addConsumers(subscription, extraConsumers);
Expand All @@ -310,7 +320,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
function addConsumers(
uint256 subscription,
address[] calldata consumers
) external onlySubscriptionOwner(subscription) {
) external forbidDatasetOwner onlySubscriptionOwner(subscription) {
_addConsumers(subscription, consumers);
}

Expand All @@ -325,7 +335,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
function removeConsumers(
uint256 subscription,
address[] calldata consumers
) external onlySubscriptionOwner(subscription) {
) external forbidDatasetOwner onlySubscriptionOwner(subscription) {
_removeConsumers(subscription, consumers);
}

Expand All @@ -343,7 +353,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
uint256 subscription,
address[] calldata oldConsumers,
address[] calldata newConsumers
) external onlySubscriptionOwner(subscription) {
) external forbidDatasetOwner onlySubscriptionOwner(subscription) {
_replaceConsumers(subscription, oldConsumers, newConsumers);
}

Expand Down Expand Up @@ -461,6 +471,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
revert MAX_CONSUMERS_ADDITION_REACHED(sd.paidConsumers, sd.consumers.length() + consumers.length);
for (uint256 i; i < consumers.length; ) {
address consumer = consumers[i];
if (dataset.ownerOf(datasetId) == consumer) revert DATASET_OWNER_FORBIDDEN(consumer);
bool added = sd.consumers.add(consumer);
if (added) {
_consumerSubscriptions[consumer].add(subscription);
Expand Down Expand Up @@ -522,6 +533,7 @@ abstract contract GenericSingleDatasetSubscriptionManager is
revert CONSUMER_NOT_FOUND(subscription, consumer);
}
consumer = newConsumers[i];
if (dataset.ownerOf(datasetId) == consumer) revert DATASET_OWNER_FORBIDDEN(consumer);
bool added = sd.consumers.add(consumer);
if (added) {
_consumerSubscriptions[consumer].add(subscription);
Expand Down
225 changes: 221 additions & 4 deletions tests/SubscriptionManager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
FragmentNFT,
VerifierManager,
} from '@typechained';
import { expect } from 'chai';
import { expect, use } from 'chai';
import { ZeroAddress, ZeroHash, parseEther, parseUnits } from 'ethers';
import { deployments, ethers, network } from 'hardhat';
import { v4 as uuidv4 } from 'uuid';
Expand Down Expand Up @@ -377,6 +377,41 @@ export default async function suite(): Promise<void> {
).to.emit(DatasetSubscriptionManager_, 'SubscriptionPaid');
});

it('Should revert if dataset owner tries to call subscribe()', async function () {
await DatasetDistributionManager_.connect(users_.datasetOwner).setTagWeights(
[ZeroHash],
[parseUnits('1', 18)]
);

await DatasetDistributionManager_.connect(users_.datasetOwner).setDatasetOwnerPercentage(
ethers.parseUnits('0.01', 18)
);

const feeAmount = parseUnits('0.00864', 18); // totalFee for 1 day & 1 consumer :: 0.00864 * 1 * 1 = 0.00864

await DatasetSubscriptionManager_.connect(users_.datasetOwner).setFee(
await users_.datasetOwner.Token!.getAddress(),
feeAmount
);

const [, maxSubscriptionFee] = await DatasetSubscriptionManager_.subscriptionFee(
datasetId_,
1,
1
);

await expect(
DatasetSubscriptionManager_.connect(users_.datasetOwner).subscribe(
datasetId_,
1, // 1 day
1,
maxSubscriptionFee
)
)
.to.be.revertedWithCustomError(DatasetSubscriptionManager_, 'DATASET_OWNER_FORBIDDEN')
.withArgs(users_.datasetOwner.address);
});

it('Should revert data set subscription if max fee is too low', async function () {
await DatasetDistributionManager_.connect(users_.datasetOwner).setTagWeights(
[ZeroHash],
Expand Down Expand Up @@ -636,12 +671,87 @@ export default async function suite(): Promise<void> {
DatasetSubscriptionManager_.connect(users_.subscriber).subscribeAndAddConsumers(
datasetId_,
1,
[users_.subscriber.address, users_.datasetOwner.address],
[users_.subscriber.address, users_.secondSubscriber.address],
maxSubscriptionFee
)
).to.emit(DatasetSubscriptionManager_, 'SubscriptionPaid');
});

it('Should revert if dataset owner tries to call subscribeAndAddConsumers()', async function () {
await DatasetDistributionManager_.connect(users_.datasetOwner).setTagWeights(
[ZeroHash],
[parseUnits('1', 18)]
);

await DatasetDistributionManager_.connect(users_.datasetOwner).setDatasetOwnerPercentage(
ethers.parseUnits('0.01', 18)
);

const feeAmount = parseUnits('0.00864', 18); // totalSubscriptionFee for 1 day & 2 consumers :: 0.00864 * 1 * 2 = 0.01728d

await DatasetSubscriptionManager_.connect(users_.datasetOwner).setFee(
await users_.datasetOwner.Token!.getAddress(),
feeAmount
);

const [, maxSubscriptionFee] = await DatasetSubscriptionManager_.subscriptionFee(
datasetId_,
1,
2
);

await expect(
DatasetSubscriptionManager_.connect(users_.datasetOwner).subscribeAndAddConsumers(
datasetId_,
1,
[users_.subscriber.address, users_.datasetOwner.address],
maxSubscriptionFee
)
)
.to.be.revertedWithCustomError(DatasetSubscriptionManager_, 'DATASET_OWNER_FORBIDDEN')
.withArgs(users_.datasetOwner.address);
});

it('Should revert if subscriber tries to add the dataset owner to the data set subscription', async function () {
await DatasetDistributionManager_.connect(users_.datasetOwner).setTagWeights(
[ZeroHash],
[parseUnits('1', 18)]
);

await users_.subscriber.Token!.approve(
await DatasetSubscriptionManager_.getAddress(),
parseUnits('0.01728', 18)
);

await DatasetDistributionManager_.connect(users_.datasetOwner).setDatasetOwnerPercentage(
ethers.parseUnits('0.01', 18)
);

const feeAmount = parseUnits('0.00864', 18); // totalSubscriptionFee for 1 day & 2 consumers :: 0.00864 * 1 * 2 = 0.01728d

await DatasetSubscriptionManager_.connect(users_.datasetOwner).setFee(
await users_.datasetOwner.Token!.getAddress(),
feeAmount
);

const [, maxSubscriptionFee] = await DatasetSubscriptionManager_.subscriptionFee(
datasetId_,
1,
2
);

await expect(
DatasetSubscriptionManager_.connect(users_.subscriber).subscribeAndAddConsumers(
datasetId_,
1,
[users_.subscriber.address, users_.datasetOwner.address],
maxSubscriptionFee
)
)
.to.be.revertedWithCustomError(DatasetSubscriptionManager_, 'DATASET_OWNER_FORBIDDEN')
.withArgs(users_.datasetOwner.address);
});

it('Should revert if subscriber tries to subscribe to the same data set', async function () {
await DatasetDistributionManager_.connect(users_.datasetOwner).setTagWeights(
[ZeroHash],
Expand Down Expand Up @@ -767,8 +877,6 @@ export default async function suite(): Promise<void> {
let snapshotId = await DatasetFragment.currentSnapshotId();
let [tagsFromFragment, counts] = await DatasetFragment.tagCountAt(snapshotId);

console.log('tagsFromFragment :>> ', tagsFromFragment);

expect(tagsFromFragment.includes(tag1)).to.be.true;
expect(tagsFromFragment.includes(tag2)).to.be.true;
expect(tagsFromFragment.includes(tag3)).to.be.true;
Expand Down Expand Up @@ -855,6 +963,15 @@ export default async function suite(): Promise<void> {
await ethers.provider.send('evm_revert', [snap]);
});

it('Should dataset owner be subscribed to its own dataset', async function () {
expect(
await DatasetSubscriptionManager_.isSubscriptionPaidFor(
datasetId_,
users_.datasetOwner.address
)
).to.be.true;
});

it('Should consumer be added multiple times to a subscription', async () => {
await DatasetSubscriptionManager_.connect(users_.subscriber).addConsumers(subscriptionId_, [
users_.consumer.address,
Expand Down Expand Up @@ -977,6 +1094,17 @@ export default async function suite(): Promise<void> {
).to.be.false;
});

it('Should revert if dataset owner tries to call removeConsumers()', async () => {
await expect(
DatasetSubscriptionManager_.connect(users_.datasetOwner).removeConsumers(
subscriptionId_,
[users_.consumer.address]
)
)
.to.be.revertedWithCustomError(DatasetSubscriptionManager_, 'DATASET_OWNER_FORBIDDEN')
.withArgs(users_.datasetOwner.address);
});

it('Should revert subscription owner remove consumers if subscription does not exists', async () => {
const wrongSubscriptionId = 12312312312n;

Expand Down Expand Up @@ -1040,6 +1168,51 @@ export default async function suite(): Promise<void> {
).to.be.true;
});

it('Should revert if subscription owner tries to replace consumers with dataset owner from the subscription', async () => {
await DatasetSubscriptionManager_.connect(users_.subscriber).addConsumers(subscriptionId_, [
users_.consumer.address,
]);

await expect(
DatasetSubscriptionManager_.connect(users_.subscriber).replaceConsumers(
subscriptionId_,
[users_.consumer.address],
[users_.datasetOwner.address]
)
)
.to.be.revertedWithCustomError(DatasetSubscriptionManager_, 'DATASET_OWNER_FORBIDDEN')
.withArgs(users_.datasetOwner.address);
});

it('Should revert if dataset owner tries to call replaceConsumers()', async () => {
await DatasetSubscriptionManager_.connect(users_.subscriber).addConsumers(subscriptionId_, [
users_.consumer.address,
]);

expect(
await DatasetSubscriptionManager_.isSubscriptionPaidFor(
datasetId_,
users_.consumer.address
)
).to.be.true;
expect(
await DatasetSubscriptionManager_.isSubscriptionPaidFor(
datasetId_,
users_.secondConsumer.address
)
).to.be.false;

await expect(
DatasetSubscriptionManager_.connect(users_.datasetOwner).replaceConsumers(
subscriptionId_,
[users_.consumer.address],
[users_.secondConsumer.address]
)
)
.to.be.revertedWithCustomError(DatasetSubscriptionManager_, 'DATASET_OWNER_FORBIDDEN')
.withArgs(users_.datasetOwner.address);
});

it('Should revert subscription owner consumers replace if old consumers and new consumers does not match length', async () => {
await DatasetSubscriptionManager_.connect(users_.subscriber).addConsumers(subscriptionId_, [
users_.consumer.address,
Expand Down Expand Up @@ -1129,6 +1302,50 @@ export default async function suite(): Promise<void> {
).to.emit(DatasetSubscriptionManager_, 'SubscriptionPaid');
});

it('Should revert if dataset owner tries to call extendSubscription()', async () => {
await time.increase(constants.ONE_DAY * 3);

const [, maxSubscriptionFee] = await DatasetSubscriptionManager_.subscriptionFee(
datasetId_,
7,
1
);

await expect(
DatasetSubscriptionManager_.connect(users_.datasetOwner).extendSubscription(
subscriptionId_,
7,
0,
maxSubscriptionFee
)
)
.to.be.revertedWithCustomError(DatasetSubscriptionManager_, 'DATASET_OWNER_FORBIDDEN')
.withArgs(users_.datasetOwner.address);
});

it('Should revert if dataset owner tries to call extendSubscriptionAndAddExtraConsumers()', async () => {
await time.increase(constants.ONE_DAY * 3);

const [, maxSubscriptionFee] = await DatasetSubscriptionManager_.subscriptionFee(
datasetId_,
7,
1
);

await expect(
DatasetSubscriptionManager_.connect(
users_.datasetOwner
).extendSubscriptionAndAddExtraConsumers(
subscriptionId_,
7,
[users_.secondSubscriber.address],
maxSubscriptionFee
)
)
.to.be.revertedWithCustomError(DatasetSubscriptionManager_, 'DATASET_OWNER_FORBIDDEN')
.withArgs(users_.datasetOwner.address);
});

it('Should subscriber extend his subscription and add consumers if expired', async () => {
await time.increase(constants.ONE_DAY * 3);

Expand Down
Loading