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

Duplicate Internal entries in a proposal JSON are filtered out #4380

Open
Rigorously opened this issue Feb 14, 2025 · 7 comments
Open

Duplicate Internal entries in a proposal JSON are filtered out #4380

Rigorously opened this issue Feb 14, 2025 · 7 comments
Labels
bug Something isn't working

Comments

@Rigorously
Copy link

Rigorously commented Feb 14, 2025

Duplicate entries with the same amount and target in a PGF proposal are filtered out.

Some Donor Drop donors messed up their transactions and were not registered correctly. This has been corrected manually, resulting in multiple allocations.

One donor:
6,777 NAM qualified
1,667 NAM correction 1
1,667 NAM correction 2

Balance after the proposal passed was 8,334 NAM instead of the expected 10,000 NAM, because correction 2 was filtered out.

      {
        "Internal": {
          "amount": "6667000000",
          "target": "tnam1qzkx2k6zxjcdxf5tz4ep55mj4ll2kq7wsyrd5k5u"
        }
      },
      {
        "Internal": {
          "amount": "1667000000",
          "target": "tnam1qzkx2k6zxjcdxf5tz4ep55mj4ll2kq7wsyrd5k5u"
        }
      },
      {
        "Internal": {
          "amount": "1667000000",
          "target": "tnam1qzkx2k6zxjcdxf5tz4ep55mj4ll2kq7wsyrd5k5u"
        }
      },

https://explorer75.org/namada-housefire/accounts/tnam1qzkx2k6zxjcdxf5tz4ep55mj4ll2kq7wsyrd5k5u

See also: https://discord.com/channels/833618405537218590/1316142427672809514/1339775312816509041

@Rigorously Rigorously added the bug Something isn't working label Feb 14, 2025
@Rigorously
Copy link
Author

Rigorously commented Feb 14, 2025

Specification requires a HashSet, while an Array was supplied.

Image

 "data": {
    "continuous": [],
    "retro": [
      {
        "Internal": {
          "amount": "1000000000",
          "target": "tnam1qp4d57kez3s2py6e5z5mc4cxjqj3ytpzqqrm5jsc"
        }
      },
      {
        "Internal": {
          "amount": "10000000000",
          "target": "tnam1qzdnlmyj6ce7dle98pnjdtv4vth4a075hy08w7d3"
        }

@Fraccaman
Copy link
Member

Fraccaman commented Feb 14, 2025

This is done on purpose, the protocol doesn't accept duplicated entries to avoid weird error related to ordering. Just merge all the entries with the same address to a single entry.

@Rigorously
Copy link
Author

This is done on purpose, the protocol doesn't accept duplicated entries to avoid weird error related to ordering. Just merge all the entries with the same address to a single entry.

This can be fixed on the processing side, letting Rust merge duplicate entries if this behavior is wanted (maybe with --force), or reject the proposal if not.

Or on the input side, adding a key to each Internal item manually, but then the parser needs to be changed too.

The current method of silently ignoring duplicates is the worst, as the proposer might never have noticed.

@Rigorously
Copy link
Author

Result of brainstorming with @zenodeapp: using target tnam address as key for the Internal object forces the proposer to merge entries.

@Fraccaman
Copy link
Member

we can validate and reject on the client side sure

@brentstone
Copy link
Collaborator

I'd be in favor of some kind of validation to enforce that there is only one value per provided Address. I say this without having looked at the code specifically yet, but seems easy. If not, then the tx should fail with error imo. It should really be on the proposer to construct a good proposal whose data can reliably be human-readable and simply digested imo as well.

@toixa
Copy link

toixa commented Feb 16, 2025

GM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants