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.

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

Add Transaction Permissioning Hook to PermissioningService Interface #7952

Draft
wants to merge 28 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
1724426
commit
vaidikcode Nov 27, 2024
858c8ee
addressing comments
vaidikcode Nov 28, 2024
dd498e7
updating api hash
vaidikcode Nov 28, 2024
aae1526
fixing the issue of implementing the permission service class in pr w…
vaidikcode Nov 28, 2024
2509d7d
add tests
vaidikcode Nov 28, 2024
6c7b463
change log
vaidikcode Nov 28, 2024
4a0c5dd
fix
vaidikcode Nov 28, 2024
2d51ec2
fix
vaidikcode Nov 28, 2024
7851707
Merge remote-tracking branch 'origin/#7835' into #7835
vaidikcode Nov 28, 2024
da2c3de
fix
vaidikcode Nov 28, 2024
dd44cca
fix
vaidikcode Nov 28, 2024
549db1f
fix
vaidikcode Dec 3, 2024
7f75593
Merge branch 'hyperledger:main' into #7835
vaidikcode Dec 3, 2024
d77f294
refactor
vaidikcode Dec 3, 2024
245e8ef
Merge remote-tracking branch 'origin/#7835' into #7835
vaidikcode Dec 3, 2024
b51669b
Merge branch 'main' into #7835
vaidikcode Dec 4, 2024
94620ed
fix
vaidikcode Dec 9, 2024
531ff5d
Merge remote-tracking branch 'origin/#7835' into #7835
vaidikcode Dec 9, 2024
ae8f3c8
Merge branch 'main' into #7835
vaidikcode Dec 9, 2024
5d815c2
spotless
vaidikcode Dec 10, 2024
9cfc510
Merge remote-tracking branch 'origin/#7835' into #7835
vaidikcode Dec 10, 2024
80c832c
compile error fix
vaidikcode Dec 11, 2024
b74a3e1
Merge branch 'main' into #7835
macfarla Dec 11, 2024
0b23b89
minor fix
vaidikcode Dec 11, 2024
de4ee35
Merge remote-tracking branch 'origin/#7835' into #7835
vaidikcode Dec 11, 2024
040857a
minor fix
vaidikcode Dec 11, 2024
a037c07
final
macfarla Dec 11, 2024
028c103
Merge branch '#7835' of github.com:vaidikcode/besu into #7835
macfarla Dec 11, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
- Fast Sync

### Additions and Improvements
- Add support for registeringTransactionPermissionProvider in Plugin API to define and validate transaction rules [#7952](https://github.com/hyperledger/besu/pull/7952)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Add support for registeringTransactionPermissionProvider in Plugin API to define and validate transaction rules [#7952](https://github.com/hyperledger/besu/pull/7952)
- Add support for transaction permissioning rules in Plugin API [#7952](https://github.com/hyperledger/besu/pull/7952)

- Fine tune already seen txs tracker when a tx is removed from the pool [#7755](https://github.com/hyperledger/besu/pull/7755)
- Support for enabling and configuring TLS/mTLS in WebSocket service. [#7854](https://github.com/hyperledger/besu/pull/7854)
- Create and publish Besu BOM (Bill of Materials) [#7615](https://github.com/hyperledger/besu/pull/7615)
Expand Down
1 change: 1 addition & 0 deletions acceptance-tests/test-plugins/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ dependencies {
implementation project(':ethereum:rlp')
implementation project(':ethereum:api')
implementation project(':plugin-api')
implementation project(':ethereum:permissioning')
implementation 'com.google.auto.service:auto-service'
implementation 'info.picocli:picocli'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.hyperledger.besu.plugin.services.PermissioningService;
import org.hyperledger.besu.plugin.services.PicoCLIOptions;


import com.google.auto.service.AutoService;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -76,6 +77,17 @@ public void beforeExternalServices() {
}
return true;
});

service.registerTransactionPermissioningProvider(
transaction -> {
long configuredGasLimitThreshold = 12000L;
long gasLimit = transaction.getGasLimit();
LOG.info("Transaction gas limit: {} | Configured threshold: {} ", gasLimit, configuredGasLimitThreshold);
return gasLimit > configuredGasLimitThreshold;
}
);


}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,17 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;

public class PermissioningPluginTest extends AcceptanceTestBase {
private BesuNode minerNode;

private BesuNode aliceNode;
private BesuNode bobNode;
private BesuNode charlieNode;

private static final long GAS_LIMIT_THRESHOLD = 12000L;

@BeforeEach
public void setUp() throws Exception {
minerNode = besu.create(createNodeBuilder().name("miner").build());
Expand Down Expand Up @@ -96,4 +100,20 @@ public void transactionsAreNotSendToBlockPendingTransactionsNode() {
charlieNode.verify(txPoolConditions.notInTransactionPool(txHash));
minerNode.verify(txPoolConditions.inTransactionPool(txHash));
}

@Test
public void testGasLimitLogic() {
final long transactionGasLimit = 10000L;
boolean isTransactionPermitted = checkTransactionGasLimit(transactionGasLimit);

assertThat(isTransactionPermitted).isTrue();
}

private boolean checkTransactionGasLimit(long gasLimit) {
if (gasLimit > GAS_LIMIT_THRESHOLD) {
return true;
} else {
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,27 @@
*/
package org.hyperledger.besu.services;

import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.permissioning.account.TransactionPermissioningProvider;
import org.hyperledger.besu.plugin.services.PermissioningService;
import org.hyperledger.besu.plugin.services.permissioning.NodeConnectionPermissioningProvider;
import org.hyperledger.besu.plugin.services.permissioning.NodeMessagePermissioningProvider;

import java.util.ArrayList;
import java.util.List;
import javax.inject.Inject;

import com.google.common.collect.Lists;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** The Permissioning service implementation. */
public class PermissioningServiceImpl implements PermissioningService {
private static final Logger LOG = LoggerFactory.getLogger(PermissioningServiceImpl.class);

private final List<NodeConnectionPermissioningProvider> connectionPermissioningProviders =
Lists.newArrayList();
private final List<TransactionPermissioningProvider> transactionPermissioningProviders = new ArrayList<>();

/** Default Constructor. */
@Inject
Expand All @@ -39,6 +46,12 @@ public void registerNodePermissioningProvider(
connectionPermissioningProviders.add(provider);
}

@Override
public void registerTransactionPermissioningProvider(TransactionPermissioningProvider provider) {
transactionPermissioningProviders.add(provider);
LOG.info("Registered new transaction permissioning provider.");
}

/**
* Gets connection permissioning providers.
*
Expand All @@ -65,4 +78,20 @@ public void registerNodeMessagePermissioningProvider(
public List<NodeMessagePermissioningProvider> getMessagePermissioningProviders() {
return messagePermissioningProviders;
}

/**
* Gets transaction rules.
*
* @return if the transaction is valid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return if the transaction is valid
* @return whether the transaction is permitted

*/
public boolean isTransactionPermitted(Transaction transaction) {
for (TransactionPermissioningProvider provider : transactionPermissioningProviders) {
if (!provider.isPermitted(transaction)) {
LOG.debug("Transaction {} not permitted by one of the providers.", transaction.getHash());
return false;
}
}
LOG.debug("Transaction {} permitted.", transaction.getHash());
return true;
}
}
2 changes: 1 addition & 1 deletion plugin-api/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Calculated : ${currentHash}
tasks.register('checkAPIChanges', FileStateChecker) {
description = "Checks that the API for the Plugin-API project does not change without deliberate thought"
files = sourceSets.main.allJava.files
knownHash = 'f6fi+lsYVZtFjmGOyiMPPCfNDie4SIPpj6HVgXRxF8Q='
knownHash = '3LJPysmUYg0xxYVX7SkUs0qD9WUzOm45/+8RnB2DpHI='
}
check.dependsOn('checkAPIChanges')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
package org.hyperledger.besu.plugin.services;

import org.hyperledger.besu.ethereum.permissioning.account.TransactionPermissioningProvider;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will need some refactoring to move this TransactionPermissioningProvider interface into the plugin package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your feedback! Are you referring to this location: plugin-api/src/main/java/org/hyperledger/besu/plugin/services/permissioning/?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a quick question: Since the TransactionPermissioningProvider is implemented by both the AccountLocalConfigPermissioningController and the TransactionSmartContractPermissioningController, what would be the best way to move forward with this? Your guidance would be greatly appreciated!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have a look at how NodeMessagePermissioningProvider (functional interface) is used, since that's already exposed to plugins and used elsewhere in besu. It's a common pattern in besu to have an interface in the plugin-api package which is imported and implemented by classes where needed. BlockHeader is another good example but you can find lots of examples in the plugin-api package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a circular dependency issue caused by importing the Transaction class into the TransactionPermissioningProvider interface. introducing a lightweight abstraction (like a DTO or a subset of transaction properties) for permissioning logic might be a good solution. wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it help if the TransactionPermissioningProvider uses the org.hyperledger.besu.datatypes.Transaction interface instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much. It works perfectly!.

import org.hyperledger.besu.plugin.services.permissioning.NodeConnectionPermissioningProvider;
import org.hyperledger.besu.plugin.services.permissioning.NodeMessagePermissioningProvider;

Expand All @@ -38,6 +39,13 @@ public interface PermissioningService extends BesuService {
*/
void registerNodePermissioningProvider(NodeConnectionPermissioningProvider provider);

/**
* Registers a callback for transaction permission.
*
* @param provider The provider to register
*/
void registerTransactionPermissioningProvider(TransactionPermissioningProvider provider);

/**
* Registers a callback to allow the interception of a devp2p message sending request
*
Expand Down