Skip to content

feat(pubkey-banning): expirable bans#2455

Merged
mariocynicys merged 4 commits intodevfrom
timed-bans
May 19, 2025
Merged

feat(pubkey-banning): expirable bans#2455
mariocynicys merged 4 commits intodevfrom
timed-bans

Conversation

@onur-ozkan
Copy link
Copy Markdown

@onur-ozkan onur-ozkan commented May 14, 2025

Implements expirable public key banning. For swaps public keys will be banned for one hour, which should be a sufficient penalty (and can be easily increased if needed). Manual bans will default to permanent but this can be changed by including the duration_min parameter in the request.

Fixes #1985

Docs issue: GLEECBTC/komodo-docs-mdx#491

Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Copy link
Copy Markdown
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

neat!
looks good other than the small bound on ban duration via rpc.

Copy link
Copy Markdown

@BigFish2086 BigFish2086 May 15, 2025

Choose a reason for hiding this comment

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

only bumping the version of timed-map on my side causes another changes in the Cargo.lock file in the signature carte version, for some reason cargo changed its version and it started using signature 2.2.0 that caused the following error:

error[E0405]: cannot find trait `Signature` in crate `signature`
   --> /home/bigfish/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ed25519-1.5.2/src/lib.rs:335:17
    |
335 | impl signature::Signature for Signature {
    |                 ^^^^^^^^^ not found in `signature`

For more information about this error, try `rustc --explain E0405`.
error: could not compile `ed25519` (lib) due to previous error
Cargo.lock Diff
@@ -1125,7 +1125,7 @@ dependencies = [
"rand_core 0.6.4",
"serde",
"serde_json",
- "signature 2.2.0",
+ "signature",
"subtle-encoding",
"tendermint",
"thiserror",
@@ -1680,7 +1680,7 @@ dependencies = [
"digest 0.10.7",
"elliptic-curve",
"rfc6979",
- "signature 2.2.0",
+ "signature",
"spki",
]

@@ -1691,7 +1691,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1e9c280362032ea4203659fc489832d0204ef09f247a0506f170dafcac08c369"
dependencies = [
"serde",
- "signature 1.4.0",
+ "signature",
]

[[package]]
@@ -1701,7 +1701,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "115531babc129696a58c64a4fef0a8bf9e9698629fb97e9e40767d235cfbcd53"
dependencies = [
"pkcs8",
- "signature 2.2.0",
+ "signature",
]

[[package]]
@@ -6326,12 +6326,6 @@ dependencies = [
"libc",
]

-[[package]]
-name = "signature"
-version = "1.4.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "02658e48d89f2bec991f9a78e69cfa4c316f8d6a6c4ec12fae1aeb263d486788"
-
[[package]]
name = "signature"
version = "2.2.0"
@@ -6807,7 +6801,7 @@ dependencies = [
"serde_json",
"serde_repr",
"sha2 0.10.7",
- "signature 2.2.0",
+ "signature",
"subtle",
"subtle-encoding",
"tendermint-proto",
@@ -6969,11 +6963,12 @@ dependencies = [

[[package]]
name = "timed-map"
-version = "1.3.1"
+version = "1.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "07be2341cfbd1b8b9a84eb9212476ea383ef5cddeb85fa3ef89dc66666196619"
+checksum = "ac74a5331850dc3b08de854b57674af757b6e286e7ef930baf71e0a196f53790"
dependencies = [
"rustc-hash",
+ "serde",
"web-time",
]

So, I wonder how you solved it? or it's only a problem on my side!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Cargo.lock usually requires manual editing on our project, unfortunately.

Copy link
Copy Markdown

@borngraced borngraced May 16, 2025

Choose a reason for hiding this comment

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

git checkout Cargo.lock and try compiling again.

last resort(revert signature update):
cargo update -p signature@2.2.0 --precise 1.4.0

cc. @BigFish2086

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Running git checkout Cargo.lock and compiling gave the same error.

And the second command, cargo update -p signature@2.2.0 --precise 1.4.0, shows different error message:

error: failed to select a version for the requirement `signature = "^2"`
candidate versions found which didn't match: 1.4.0
location searched: crates.io index
required by package `cosmrs v0.16.0`
    ... which satisfies dependency `cosmrs = "^0.16"` (locked to 0.16.0) of package `coins v0.1.0 (/home/bigfish/wsp/kdf/komodo/mm2src/coins)`
    ... which satisfies path dependency `coins` (locked to 0.1.0) of package `coins_activation v0.1.0 (/home/bigfish/wsp/kdf/komodo/mm2src/coins_activation)`
perhaps a crate was updated and forgotten to be re-vendored?

can it just be a cargo problem?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ah, I think rebasing on latest dev should work

  • git checkout Cargo.lock to its original state
  • remove the package you upgrade which I assume is timed-map
  • rebase your branch with on dev

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

try cargo cleaning after checking out Cargo.lock

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I thought you were manually updating the packages. If you just want to build/test this branch, do this:

git reset --hard de6816dc406f4d6ebbf9d88103dd94e0bc60c2a3`

which will revoke and delete everything you have and make it synced with the latest commit in this PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If you just want to build/test this branch

nah. he was working on the same feat in parallel but faced this issue when he tried to bump timed map. not related to testing PR.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Copy link
Copy Markdown
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM!

@mariocynicys mariocynicys merged commit 66d3b2a into dev May 19, 2025
21 of 25 checks passed
@mariocynicys mariocynicys deleted the timed-bans branch May 19, 2025 11:28
dimxy pushed a commit that referenced this pull request May 28, 2025
* dev: (29 commits)
  fix(p2pk): validate expected pubkey correctly for p2pk inputs (#2408)
  chore(docs): update old urls referencing atomicdex or old docs pages (#2428)
  improvement(p2p): remove hardcoded seeds (#2439)
  fix(evm-api): find enabled erc20 token using platform ticker (#2445)
  chore(docs): add DeepWiki badge to README (#2463)
  chore(core): organize deps using workspace.dependencies (#2449)
  feat(db-arch): more dbdir to address_dir replacements (#2398)
  chore(build-artifacts): remove duplicated mm2 build artifacts (#2448)
  feat(pubkey-banning): expirable bans (#2455)
  fix(eth-balance-events): serialize eth address using AddrToString (#2440)
  chore(deps): remove base58 and replace it completely with bs58 (#2427)
  feat(tron): initial groundwork for full TRON integration (#2425)
  fix(UTXO): improve tx fee calculation and min relay fee handling (#2316)
  deps(timed-map): bump to 1.3.1 (#2413)
  improvement(tendermint): safer IBC channel handler (#2298)
  chore(release): complete v2.4.0-beta changelogs  (#2436)
  fix(event-streaming): initial addresses registration in utxo balance streaming (#2431)
  improvement(watchers): re-write use-watchers handling (#2430)
  fix(evm): make withdraw_nft work in HD mode (#2424)
  feat(taproot): support parsing taproot output address types
  ...
@smk762
Copy link
Copy Markdown

smk762 commented Jun 5, 2025

@onur-ozkan While testing this, I encountered some cases in where a ban does not expire until another pubkey is banned.

To replicate:

  • ban a pubkey for 1 minute
{
    "userpass": "{{userpass}}",
    "method": "ban_pubkey",
    "pubkey": "15d9c51c657ab1be4ae9d3ab6e76a619d3bccfe830d5363fa168424c0d044732",
    "reason": "1 duration_min test @ 0901",
    "duration_min": 1
  }
  • wait 10 mins
  • call list_banned_pubkeys
  • see original pubkey is still banned in list_banned_pubkeys response.
  • ban a second pubkey
  • see original pubkey is no longer in list_banned_pubkeys response.

This occurs both when the second pubkey includes the duration_min param and when it does not.

I'll convert this to an issue as the PR has been merged. Docs PR can still be reviewed as the instructions should not change when this bug is squashed.

dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request Jun 8, 2025
* lr-swap-wip: (37 commits)
  fix custom token error name
  fix getting chain_id from protocol_data
  refactor (review): use dedicated large error cfg, add new fn to FromApiValueError, fix TODO, use experimental namespace for lr rpc, more Ticker alias
  feat(walletconnect): add WalletConnect v2 support for EVM and Cosmos (GLEECBTC#2223)
  feat(ibc-routing-part-1): supporting entire Cosmos network for swaps (GLEECBTC#2459)
  fix(test): fix HD Wallet message signing tests (GLEECBTC#2474)
  improvement(builds): enable static CRT linking for MSVC builds (GLEECBTC#2464)
  feat(wallet): implement HD multi-address support for message signing (GLEECBTC#2432)
  fix(p2pk): validate expected pubkey correctly for p2pk inputs (GLEECBTC#2408)
  chore(docs): update old urls referencing atomicdex or old docs pages (GLEECBTC#2428)
  improvement(p2p): remove hardcoded seeds (GLEECBTC#2439)
  fix(evm-api): find enabled erc20 token using platform ticker (GLEECBTC#2445)
  chore(docs): add DeepWiki badge to README (GLEECBTC#2463)
  chore(core): organize deps using workspace.dependencies (GLEECBTC#2449)
  feat(db-arch): more dbdir to address_dir replacements (GLEECBTC#2398)
  chore(build-artifacts): remove duplicated mm2 build artifacts (GLEECBTC#2448)
  feat(pubkey-banning): expirable bans (GLEECBTC#2455)
  fix(eth-balance-events): serialize eth address using AddrToString (GLEECBTC#2440)
  chore(deps): remove base58 and replace it completely with bs58 (GLEECBTC#2427)
  feat(tron): initial groundwork for full TRON integration (GLEECBTC#2425)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants